* [PATCH 2/4] drm/i915/crt: split compute_config hook by platforms
2017-10-12 16:05 [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI Jani Nikula
@ 2017-10-12 16:05 ` Jani Nikula
2017-10-12 16:23 ` Ville Syrjälä
2017-10-12 16:05 ` [PATCH 3/4] drm/i915: push crtc compute clock to encoder compute config on DDI Jani Nikula
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2017-10-12 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Daniel Vetter
Only the DDI hook has some actual content.
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_crt.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 668e8c3e791d..437339f5d098 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -344,10 +344,25 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config,
struct drm_connector_state *conn_state)
{
+ return true;
+}
+
+static bool pch_crt_compute_config(struct intel_encoder *encoder,
+ struct intel_crtc_state *pipe_config,
+ struct drm_connector_state *conn_state)
+{
+ pipe_config->has_pch_encoder = true;
+
+ return true;
+}
+
+static bool hsw_crt_compute_config(struct intel_encoder *encoder,
+ struct intel_crtc_state *pipe_config,
+ struct drm_connector_state *conn_state)
+{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
- if (HAS_PCH_SPLIT(dev_priv))
- pipe_config->has_pch_encoder = true;
+ pipe_config->has_pch_encoder = true;
/* LPT FDI RX only supports 8bpc. */
if (HAS_PCH_LPT(dev_priv)) {
@@ -360,8 +375,7 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
}
/* FDI must always be 2.7 GHz */
- if (HAS_DDI(dev_priv))
- pipe_config->port_clock = 135000 * 2;
+ pipe_config->port_clock = 135000 * 2;
return true;
}
@@ -959,11 +973,11 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
!dmi_check_system(intel_spurious_crt_detect))
crt->base.hpd_pin = HPD_CRT;
- crt->base.compute_config = intel_crt_compute_config;
if (HAS_DDI(dev_priv)) {
crt->base.port = PORT_E;
crt->base.get_config = hsw_crt_get_config;
crt->base.get_hw_state = intel_ddi_get_hw_state;
+ crt->base.compute_config = hsw_crt_compute_config;
crt->base.pre_pll_enable = hsw_pre_pll_enable_crt;
crt->base.pre_enable = hsw_pre_enable_crt;
crt->base.enable = hsw_enable_crt;
@@ -971,9 +985,11 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
crt->base.post_disable = hsw_post_disable_crt;
} else {
if (HAS_PCH_SPLIT(dev_priv)) {
+ crt->base.compute_config = pch_crt_compute_config;
crt->base.disable = pch_disable_crt;
crt->base.post_disable = pch_post_disable_crt;
} else {
+ crt->base.compute_config = intel_crt_compute_config;
crt->base.disable = intel_disable_crt;
}
crt->base.port = PORT_NONE;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/4] drm/i915/crt: split compute_config hook by platforms
2017-10-12 16:05 ` [PATCH 2/4] drm/i915/crt: split compute_config hook by platforms Jani Nikula
@ 2017-10-12 16:23 ` Ville Syrjälä
0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2017-10-12 16:23 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx
On Thu, Oct 12, 2017 at 07:05:31PM +0300, Jani Nikula wrote:
> Only the DDI hook has some actual content.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_crt.c | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 668e8c3e791d..437339f5d098 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -344,10 +344,25 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config,
> struct drm_connector_state *conn_state)
> {
> + return true;
> +}
> +
> +static bool pch_crt_compute_config(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config,
> + struct drm_connector_state *conn_state)
> +{
> + pipe_config->has_pch_encoder = true;
> +
> + return true;
> +}
> +
> +static bool hsw_crt_compute_config(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config,
> + struct drm_connector_state *conn_state)
> +{
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>
> - if (HAS_PCH_SPLIT(dev_priv))
> - pipe_config->has_pch_encoder = true;
> + pipe_config->has_pch_encoder = true;
>
> /* LPT FDI RX only supports 8bpc. */
> if (HAS_PCH_LPT(dev_priv)) {
I believe HAS_PCH_LPT is always going to be true here. So could drop
this check as well.
Apart from that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> @@ -360,8 +375,7 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
> }
>
> /* FDI must always be 2.7 GHz */
> - if (HAS_DDI(dev_priv))
> - pipe_config->port_clock = 135000 * 2;
> + pipe_config->port_clock = 135000 * 2;
>
> return true;
> }
> @@ -959,11 +973,11 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
> !dmi_check_system(intel_spurious_crt_detect))
> crt->base.hpd_pin = HPD_CRT;
>
> - crt->base.compute_config = intel_crt_compute_config;
> if (HAS_DDI(dev_priv)) {
> crt->base.port = PORT_E;
> crt->base.get_config = hsw_crt_get_config;
> crt->base.get_hw_state = intel_ddi_get_hw_state;
> + crt->base.compute_config = hsw_crt_compute_config;
> crt->base.pre_pll_enable = hsw_pre_pll_enable_crt;
> crt->base.pre_enable = hsw_pre_enable_crt;
> crt->base.enable = hsw_enable_crt;
> @@ -971,9 +985,11 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
> crt->base.post_disable = hsw_post_disable_crt;
> } else {
> if (HAS_PCH_SPLIT(dev_priv)) {
> + crt->base.compute_config = pch_crt_compute_config;
> crt->base.disable = pch_disable_crt;
> crt->base.post_disable = pch_post_disable_crt;
> } else {
> + crt->base.compute_config = intel_crt_compute_config;
> crt->base.disable = intel_disable_crt;
> }
> crt->base.port = PORT_NONE;
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/4] drm/i915: push crtc compute clock to encoder compute config on DDI
2017-10-12 16:05 [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI Jani Nikula
2017-10-12 16:05 ` [PATCH 2/4] drm/i915/crt: split compute_config hook by platforms Jani Nikula
@ 2017-10-12 16:05 ` Jani Nikula
2017-10-12 16:29 ` Ville Syrjälä
2017-10-12 16:05 ` [PATCH 4/4] drm/i915: push shared dpll enable to encoders on DDI platforms Jani Nikula
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2017-10-12 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Daniel Vetter
With this, we can get rid of the encoder specific stuff that we had in
haswell_crtc_compute_clock(), which we throw away entirely. This also
paves the way for potential further untangling of encoder specific
checks in the depths of intel_get_shared_dpll() that plague the DDI
platforms.
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_crt.c | 7 +++++++
drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
drivers/gpu/drm/i915/intel_display.c | 21 ---------------------
3 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 437339f5d098..67f771f24608 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -361,6 +361,7 @@ static bool hsw_crt_compute_config(struct intel_encoder *encoder,
struct drm_connector_state *conn_state)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
pipe_config->has_pch_encoder = true;
@@ -377,6 +378,12 @@ static bool hsw_crt_compute_config(struct intel_encoder *encoder,
/* FDI must always be 2.7 GHz */
pipe_config->port_clock = 135000 * 2;
+ if (!intel_get_shared_dpll(crtc, pipe_config, encoder)) {
+ DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
+ pipe_name(crtc->pipe));
+ return false;
+ }
+
return true;
}
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b307b6fe1ce3..9819e51fa160 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2590,6 +2590,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
struct drm_connector_state *conn_state)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
int type = encoder->type;
int port = intel_ddi_get_encoder_port(encoder);
int ret;
@@ -2609,6 +2610,12 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
bxt_ddi_phy_calc_lane_lat_optim_mask(encoder,
pipe_config->lane_count);
+ if (!intel_get_shared_dpll(crtc, pipe_config, encoder)) {
+ DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
+ pipe_name(crtc->pipe));
+ return false;
+ }
+
return ret;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2faa938f57a3..6ed299670f27 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9011,23 +9011,6 @@ 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)
-{
- if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
- struct intel_encoder *encoder =
- intel_ddi_get_crtc_new_encoder(crtc_state);
-
- if (!intel_get_shared_dpll(crtc, crtc_state, encoder)) {
- DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
- pipe_name(crtc->pipe));
- return -EINVAL;
- }
- }
-
- return 0;
-}
-
static void cannonlake_get_ddi_pll(struct drm_i915_private *dev_priv,
enum port port,
struct intel_crtc_state *pipe_config)
@@ -14141,16 +14124,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
dev_priv->display.get_pipe_config = haswell_get_pipe_config;
dev_priv->display.get_initial_plane_config =
skylake_get_initial_plane_config;
- dev_priv->display.crtc_compute_clock =
- haswell_crtc_compute_clock;
dev_priv->display.crtc_enable = haswell_crtc_enable;
dev_priv->display.crtc_disable = haswell_crtc_disable;
} else if (HAS_DDI(dev_priv)) {
dev_priv->display.get_pipe_config = haswell_get_pipe_config;
dev_priv->display.get_initial_plane_config =
ironlake_get_initial_plane_config;
- dev_priv->display.crtc_compute_clock =
- haswell_crtc_compute_clock;
dev_priv->display.crtc_enable = haswell_crtc_enable;
dev_priv->display.crtc_disable = haswell_crtc_disable;
} else if (HAS_PCH_SPLIT(dev_priv)) {
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/4] drm/i915: push crtc compute clock to encoder compute config on DDI
2017-10-12 16:05 ` [PATCH 3/4] drm/i915: push crtc compute clock to encoder compute config on DDI Jani Nikula
@ 2017-10-12 16:29 ` Ville Syrjälä
2017-12-16 12:25 ` Chauhan, Madhav
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2017-10-12 16:29 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx
On Thu, Oct 12, 2017 at 07:05:32PM +0300, Jani Nikula wrote:
> With this, we can get rid of the encoder specific stuff that we had in
> haswell_crtc_compute_clock(), which we throw away entirely. This also
> paves the way for potential further untangling of encoder specific
> checks in the depths of intel_get_shared_dpll() that plague the DDI
> platforms.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_crt.c | 7 +++++++
> drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
> drivers/gpu/drm/i915/intel_display.c | 21 ---------------------
> 3 files changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 437339f5d098..67f771f24608 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -361,6 +361,7 @@ static bool hsw_crt_compute_config(struct intel_encoder *encoder,
> struct drm_connector_state *conn_state)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
>
> pipe_config->has_pch_encoder = true;
>
> @@ -377,6 +378,12 @@ static bool hsw_crt_compute_config(struct intel_encoder *encoder,
> /* FDI must always be 2.7 GHz */
> pipe_config->port_clock = 135000 * 2;
>
> + if (!intel_get_shared_dpll(crtc, pipe_config, encoder)) {
> + DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
> + pipe_name(crtc->pipe));
> + return false;
> + }
I'm not convinced we can do this. intel_modeset_clear_plls() hasn't even
been called yet.
> +
> return true;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index b307b6fe1ce3..9819e51fa160 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2590,6 +2590,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> struct drm_connector_state *conn_state)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> int type = encoder->type;
> int port = intel_ddi_get_encoder_port(encoder);
> int ret;
> @@ -2609,6 +2610,12 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> bxt_ddi_phy_calc_lane_lat_optim_mask(encoder,
> pipe_config->lane_count);
>
> + if (!intel_get_shared_dpll(crtc, pipe_config, encoder)) {
> + DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
> + pipe_name(crtc->pipe));
> + return false;
> + }
> +
> return ret;
>
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2faa938f57a3..6ed299670f27 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9011,23 +9011,6 @@ 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)
> -{
> - if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
> - struct intel_encoder *encoder =
> - intel_ddi_get_crtc_new_encoder(crtc_state);
> -
> - if (!intel_get_shared_dpll(crtc, crtc_state, encoder)) {
> - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> - pipe_name(crtc->pipe));
> - return -EINVAL;
> - }
> - }
> -
> - return 0;
> -}
> -
> static void cannonlake_get_ddi_pll(struct drm_i915_private *dev_priv,
> enum port port,
> struct intel_crtc_state *pipe_config)
> @@ -14141,16 +14124,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
> dev_priv->display.get_pipe_config = haswell_get_pipe_config;
> dev_priv->display.get_initial_plane_config =
> skylake_get_initial_plane_config;
> - dev_priv->display.crtc_compute_clock =
> - haswell_crtc_compute_clock;
> dev_priv->display.crtc_enable = haswell_crtc_enable;
> dev_priv->display.crtc_disable = haswell_crtc_disable;
> } else if (HAS_DDI(dev_priv)) {
> dev_priv->display.get_pipe_config = haswell_get_pipe_config;
> dev_priv->display.get_initial_plane_config =
> ironlake_get_initial_plane_config;
> - dev_priv->display.crtc_compute_clock =
> - haswell_crtc_compute_clock;
> dev_priv->display.crtc_enable = haswell_crtc_enable;
> dev_priv->display.crtc_disable = haswell_crtc_disable;
> } else if (HAS_PCH_SPLIT(dev_priv)) {
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 3/4] drm/i915: push crtc compute clock to encoder compute config on DDI
2017-10-12 16:29 ` Ville Syrjälä
@ 2017-12-16 12:25 ` Chauhan, Madhav
0 siblings, 0 replies; 16+ messages in thread
From: Chauhan, Madhav @ 2017-12-16 12:25 UTC (permalink / raw)
To: 'Ville Syrjälä', Nikula, Jani; +Cc: Daniel Vetter, intel-gfx
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Ville Syrjälä
> Sent: Thursday, October 12, 2017 9:59 PM
> To: Nikula, Jani <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 3/4] drm/i915: push crtc compute clock to
> encoder compute config on DDI
>
> On Thu, Oct 12, 2017 at 07:05:32PM +0300, Jani Nikula wrote:
> > With this, we can get rid of the encoder specific stuff that we had in
> > haswell_crtc_compute_clock(), which we throw away entirely. This also
> > paves the way for potential further untangling of encoder specific
> > checks in the depths of intel_get_shared_dpll() that plague the DDI
> > platforms.
> >
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_crt.c | 7 +++++++
> > drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
> > drivers/gpu/drm/i915/intel_display.c | 21 ---------------------
> > 3 files changed, 14 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c
> > b/drivers/gpu/drm/i915/intel_crt.c
> > index 437339f5d098..67f771f24608 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -361,6 +361,7 @@ static bool hsw_crt_compute_config(struct
> intel_encoder *encoder,
> > struct drm_connector_state *conn_state) {
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> >
> > pipe_config->has_pch_encoder = true;
> >
> > @@ -377,6 +378,12 @@ static bool hsw_crt_compute_config(struct
> intel_encoder *encoder,
> > /* FDI must always be 2.7 GHz */
> > pipe_config->port_clock = 135000 * 2;
> >
> > + if (!intel_get_shared_dpll(crtc, pipe_config, encoder)) {
> > + DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
> > + pipe_name(crtc->pipe));
> > + return false;
> > + }
>
> I'm not convinced we can do this. intel_modeset_clear_plls() hasn't even
> been called yet.
Agree.
How about calling intel_modeset_clear_plls() before encoder->compute_config()
Inside intel_modeset_pipe_config() instead of intel_modeset_checks()??
Looked at modeset code briefly, couldn’t find any user of DPLL between intel_modeset_pipe_config() and
intel_modeset_checks().
Please correct if wrong.
Regards,
Madhav
>
> > +
> > return true;
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index b307b6fe1ce3..9819e51fa160 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2590,6 +2590,7 @@ static bool intel_ddi_compute_config(struct
> intel_encoder *encoder,
> > struct drm_connector_state *conn_state)
> {
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> > int type = encoder->type;
> > int port = intel_ddi_get_encoder_port(encoder);
> > int ret;
> > @@ -2609,6 +2610,12 @@ static bool intel_ddi_compute_config(struct
> intel_encoder *encoder,
> > bxt_ddi_phy_calc_lane_lat_optim_mask(encoder,
> > pipe_config-
> >lane_count);
> >
> > + if (!intel_get_shared_dpll(crtc, pipe_config, encoder)) {
> > + DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
> > + pipe_name(crtc->pipe));
> > + return false;
> > + }
> > +
> > return ret;
> >
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 2faa938f57a3..6ed299670f27 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9011,23 +9011,6 @@ 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)
> > -{
> > - if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
> > - struct intel_encoder *encoder =
> > - intel_ddi_get_crtc_new_encoder(crtc_state);
> > -
> > - if (!intel_get_shared_dpll(crtc, crtc_state, encoder)) {
> > - DRM_DEBUG_DRIVER("failed to find PLL for pipe
> %c\n",
> > - pipe_name(crtc->pipe));
> > - return -EINVAL;
> > - }
> > - }
> > -
> > - return 0;
> > -}
> > -
> > static void cannonlake_get_ddi_pll(struct drm_i915_private *dev_priv,
> > enum port port,
> > struct intel_crtc_state *pipe_config) @@ -
> 14141,16 +14124,12
> > @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
> > dev_priv->display.get_pipe_config =
> haswell_get_pipe_config;
> > dev_priv->display.get_initial_plane_config =
> > skylake_get_initial_plane_config;
> > - dev_priv->display.crtc_compute_clock =
> > - haswell_crtc_compute_clock;
> > dev_priv->display.crtc_enable = haswell_crtc_enable;
> > dev_priv->display.crtc_disable = haswell_crtc_disable;
> > } else if (HAS_DDI(dev_priv)) {
> > dev_priv->display.get_pipe_config =
> haswell_get_pipe_config;
> > dev_priv->display.get_initial_plane_config =
> > ironlake_get_initial_plane_config;
> > - dev_priv->display.crtc_compute_clock =
> > - haswell_crtc_compute_clock;
> > dev_priv->display.crtc_enable = haswell_crtc_enable;
> > dev_priv->display.crtc_disable = haswell_crtc_disable;
> > } else if (HAS_PCH_SPLIT(dev_priv)) {
> > --
> > 2.11.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/4] drm/i915: push shared dpll enable to encoders on DDI platforms
2017-10-12 16:05 [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI Jani Nikula
2017-10-12 16:05 ` [PATCH 2/4] drm/i915/crt: split compute_config hook by platforms Jani Nikula
2017-10-12 16:05 ` [PATCH 3/4] drm/i915: push crtc compute clock to encoder compute config on DDI Jani Nikula
@ 2017-10-12 16:05 ` Jani Nikula
2017-12-18 12:31 ` Chauhan, Madhav
2017-10-12 16:17 ` [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI Ville Syrjälä
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2017-10-12 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Daniel Vetter
As we move more encoder specific stuff to encoders on DDI platforms,
follow suit with shared dpll enable. In the future, we'll need this
refactoring for further encoder specific details in the middle of the
enable sequence.
v2: drop ifs around the intel_enable_shared_dpll calls (Daniel)
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_crt.c | 2 ++
drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 3 ---
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 67f771f24608..65f28062cb51 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -255,6 +255,8 @@ static void hsw_pre_pll_enable_crt(struct intel_encoder *encoder,
WARN_ON(!intel_crtc->config->has_pch_encoder);
intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
+
+ intel_enable_shared_dpll(intel_crtc);
}
static void hsw_pre_enable_crt(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9819e51fa160..05f9db9c3d52 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2416,13 +2416,27 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
}
}
+static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
+ const struct intel_crtc_state *pipe_config,
+ const struct drm_connector_state *conn_state)
+{
+ struct drm_crtc *crtc = pipe_config->base.crtc;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+ intel_enable_shared_dpll(intel_crtc);
+}
+
static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
const struct intel_crtc_state *pipe_config,
const struct drm_connector_state *conn_state)
{
+ struct drm_crtc *crtc = pipe_config->base.crtc;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
uint8_t mask = pipe_config->lane_lat_optim_mask;
bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
+
+ intel_enable_shared_dpll(intel_crtc);
}
void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
@@ -2730,6 +2744,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
intel_encoder->enable = intel_enable_ddi;
if (IS_GEN9_LP(dev_priv))
intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
+ else
+ intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
intel_encoder->pre_enable = intel_ddi_pre_enable;
intel_encoder->disable = intel_disable_ddi;
intel_encoder->post_disable = intel_ddi_post_disable;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6ed299670f27..d9ccde0a8097 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5481,9 +5481,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
- if (intel_crtc->config->shared_dpll)
- intel_enable_shared_dpll(intel_crtc);
-
if (intel_crtc_has_dp_encoder(intel_crtc->config))
intel_dp_set_m_n(intel_crtc, M1_N1);
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] drm/i915: push shared dpll enable to encoders on DDI platforms
2017-10-12 16:05 ` [PATCH 4/4] drm/i915: push shared dpll enable to encoders on DDI platforms Jani Nikula
@ 2017-12-18 12:31 ` Chauhan, Madhav
2017-12-18 12:51 ` Jani Nikula
0 siblings, 1 reply; 16+ messages in thread
From: Chauhan, Madhav @ 2017-12-18 12:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Nikula, Jani, Daniel Vetter
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Thursday, October 12, 2017 9:36 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; Daniel Vetter
> <daniel.vetter@ffwll.ch>
> Subject: [Intel-gfx] [PATCH 4/4] drm/i915: push shared dpll enable to
> encoders on DDI platforms
>
> As we move more encoder specific stuff to encoders on DDI platforms,
> follow suit with shared dpll enable. In the future, we'll need this refactoring
> for further encoder specific details in the middle of the enable sequence.
>
> v2: drop ifs around the intel_enable_shared_dpll calls (Daniel)
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_crt.c | 2 ++
> drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++++++++++++
> drivers/gpu/drm/i915/intel_display.c | 3 ---
> 3 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c
> b/drivers/gpu/drm/i915/intel_crt.c
> index 67f771f24608..65f28062cb51 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -255,6 +255,8 @@ static void hsw_pre_pll_enable_crt(struct
> intel_encoder *encoder,
> WARN_ON(!intel_crtc->config->has_pch_encoder);
>
> intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> +
> + intel_enable_shared_dpll(intel_crtc);
> }
>
> static void hsw_pre_enable_crt(struct intel_encoder *encoder, diff --git
> a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 9819e51fa160..05f9db9c3d52 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2416,13 +2416,27 @@ static void intel_disable_ddi(struct intel_encoder
> *intel_encoder,
> }
> }
>
> +static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> + const struct intel_crtc_state *pipe_config,
> + const struct drm_connector_state
> *conn_state) {
> + struct drm_crtc *crtc = pipe_config->base.crtc;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> + intel_enable_shared_dpll(intel_crtc);
> +}
> +
Shouldn’t we add this code (and below as well) inside enable() or pre_enable() functions
of struct intel_encoder as pre_pll_enable() function should have some code prior to enabling PLL.
Regards,
Madhav
> static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config,
> const struct drm_connector_state
> *conn_state) {
> + struct drm_crtc *crtc = pipe_config->base.crtc;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> uint8_t mask = pipe_config->lane_lat_optim_mask;
>
> bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
> +
> + intel_enable_shared_dpll(intel_crtc);
> }
>
> void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp) @@ -2730,6
> +2744,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum
> port port)
> intel_encoder->enable = intel_enable_ddi;
> if (IS_GEN9_LP(dev_priv))
> intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> + else
> + intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
> intel_encoder->pre_enable = intel_ddi_pre_enable;
> intel_encoder->disable = intel_disable_ddi;
> intel_encoder->post_disable = intel_ddi_post_disable; diff --git
> a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 6ed299670f27..d9ccde0a8097 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5481,9 +5481,6 @@ static void haswell_crtc_enable(struct
> intel_crtc_state *pipe_config,
>
> intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>
> - if (intel_crtc->config->shared_dpll)
> - intel_enable_shared_dpll(intel_crtc);
> -
> if (intel_crtc_has_dp_encoder(intel_crtc->config))
> intel_dp_set_m_n(intel_crtc, M1_N1);
>
> --
> 2.11.0
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] drm/i915: push shared dpll enable to encoders on DDI platforms
2017-12-18 12:31 ` Chauhan, Madhav
@ 2017-12-18 12:51 ` Jani Nikula
2017-12-18 13:05 ` Chauhan, Madhav
0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2017-12-18 12:51 UTC (permalink / raw)
To: Chauhan, Madhav, intel-gfx; +Cc: Daniel Vetter
On Mon, 18 Dec 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Jani Nikula
>> Sent: Thursday, October 12, 2017 9:36 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; Daniel Vetter
>> <daniel.vetter@ffwll.ch>
>> Subject: [Intel-gfx] [PATCH 4/4] drm/i915: push shared dpll enable to
>> encoders on DDI platforms
>>
>> As we move more encoder specific stuff to encoders on DDI platforms,
>> follow suit with shared dpll enable. In the future, we'll need this refactoring
>> for further encoder specific details in the middle of the enable sequence.
>>
>> v2: drop ifs around the intel_enable_shared_dpll calls (Daniel)
>>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_crt.c | 2 ++
>> drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++++++++++++
>> drivers/gpu/drm/i915/intel_display.c | 3 ---
>> 3 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_crt.c
>> b/drivers/gpu/drm/i915/intel_crt.c
>> index 67f771f24608..65f28062cb51 100644
>> --- a/drivers/gpu/drm/i915/intel_crt.c
>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>> @@ -255,6 +255,8 @@ static void hsw_pre_pll_enable_crt(struct
>> intel_encoder *encoder,
>> WARN_ON(!intel_crtc->config->has_pch_encoder);
>>
>> intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
>> +
>> + intel_enable_shared_dpll(intel_crtc);
>> }
>>
>> static void hsw_pre_enable_crt(struct intel_encoder *encoder, diff --git
>> a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index 9819e51fa160..05f9db9c3d52 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -2416,13 +2416,27 @@ static void intel_disable_ddi(struct intel_encoder
>> *intel_encoder,
>> }
>> }
>>
>> +static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
>> + const struct intel_crtc_state *pipe_config,
>> + const struct drm_connector_state
>> *conn_state) {
>> + struct drm_crtc *crtc = pipe_config->base.crtc;
>> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> +
>> + intel_enable_shared_dpll(intel_crtc);
>> +}
>> +
>
> Shouldn’t we add this code (and below as well) inside enable() or pre_enable() functions
> of struct intel_encoder as pre_pll_enable() function should have some code prior to enabling PLL.
The first step should be pushing the calls down to encoder hooks while
keeping the modeset sequence as close as possible to before. After that,
we can start tweaking the order where applicable. Always make minimal
changes so you see where stuff breaks.
BR
Jani.
>
> Regards,
> Madhav
>
>> static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
>> const struct intel_crtc_state *pipe_config,
>> const struct drm_connector_state
>> *conn_state) {
>> + struct drm_crtc *crtc = pipe_config->base.crtc;
>> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> uint8_t mask = pipe_config->lane_lat_optim_mask;
>>
>> bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
>> +
>> + intel_enable_shared_dpll(intel_crtc);
>> }
>>
>> void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp) @@ -2730,6
>> +2744,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum
>> port port)
>> intel_encoder->enable = intel_enable_ddi;
>> if (IS_GEN9_LP(dev_priv))
>> intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
>> + else
>> + intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
>> intel_encoder->pre_enable = intel_ddi_pre_enable;
>> intel_encoder->disable = intel_disable_ddi;
>> intel_encoder->post_disable = intel_ddi_post_disable; diff --git
>> a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 6ed299670f27..d9ccde0a8097 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5481,9 +5481,6 @@ static void haswell_crtc_enable(struct
>> intel_crtc_state *pipe_config,
>>
>> intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>>
>> - if (intel_crtc->config->shared_dpll)
>> - intel_enable_shared_dpll(intel_crtc);
>> -
>> if (intel_crtc_has_dp_encoder(intel_crtc->config))
>> intel_dp_set_m_n(intel_crtc, M1_N1);
>>
>> --
>> 2.11.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] drm/i915: push shared dpll enable to encoders on DDI platforms
2017-12-18 12:51 ` Jani Nikula
@ 2017-12-18 13:05 ` Chauhan, Madhav
2017-12-20 11:07 ` Chauhan, Madhav
0 siblings, 1 reply; 16+ messages in thread
From: Chauhan, Madhav @ 2017-12-18 13:05 UTC (permalink / raw)
To: Nikula, Jani, intel-gfx; +Cc: Daniel Vetter
> -----Original Message-----
> From: Nikula, Jani
> Sent: Monday, December 18, 2017 6:21 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Subject: RE: [Intel-gfx] [PATCH 4/4] drm/i915: push shared dpll enable to
> encoders on DDI platforms
>
> On Mon, 18 Dec 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> >> Behalf Of Jani Nikula
> >> Sent: Thursday, October 12, 2017 9:36 PM
> >> To: intel-gfx@lists.freedesktop.org
> >> Cc: Nikula, Jani <jani.nikula@intel.com>; Daniel Vetter
> >> <daniel.vetter@ffwll.ch>
> >> Subject: [Intel-gfx] [PATCH 4/4] drm/i915: push shared dpll enable to
> >> encoders on DDI platforms
> >>
> >> As we move more encoder specific stuff to encoders on DDI platforms,
> >> follow suit with shared dpll enable. In the future, we'll need this
> >> refactoring for further encoder specific details in the middle of the enable
> sequence.
> >>
> >> v2: drop ifs around the intel_enable_shared_dpll calls (Daniel)
> >>
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_crt.c | 2 ++
> >> drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++++++++++++
> >> drivers/gpu/drm/i915/intel_display.c | 3 ---
> >> 3 files changed, 18 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_crt.c
> >> b/drivers/gpu/drm/i915/intel_crt.c
> >> index 67f771f24608..65f28062cb51 100644
> >> --- a/drivers/gpu/drm/i915/intel_crt.c
> >> +++ b/drivers/gpu/drm/i915/intel_crt.c
> >> @@ -255,6 +255,8 @@ static void hsw_pre_pll_enable_crt(struct
> >> intel_encoder *encoder,
> >> WARN_ON(!intel_crtc->config->has_pch_encoder);
> >>
> >> intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> >> +
> >> + intel_enable_shared_dpll(intel_crtc);
> >> }
> >>
> >> static void hsw_pre_enable_crt(struct intel_encoder *encoder, diff
> >> --git a/drivers/gpu/drm/i915/intel_ddi.c
> >> b/drivers/gpu/drm/i915/intel_ddi.c
> >> index 9819e51fa160..05f9db9c3d52 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -2416,13 +2416,27 @@ static void intel_disable_ddi(struct
> >> intel_encoder *intel_encoder,
> >> }
> >> }
> >>
> >> +static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> >> + const struct intel_crtc_state *pipe_config,
> >> + const struct drm_connector_state
> >> *conn_state) {
> >> + struct drm_crtc *crtc = pipe_config->base.crtc;
> >> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >> +
> >> + intel_enable_shared_dpll(intel_crtc);
> >> +}
> >> +
> >
> > Shouldn’t we add this code (and below as well) inside enable() or
> > pre_enable() functions of struct intel_encoder as pre_pll_enable() function
> should have some code prior to enabling PLL.
>
> The first step should be pushing the calls down to encoder hooks while
> keeping the modeset sequence as close as possible to before. After that, we
> can start tweaking the order where applicable. Always make minimal
> changes so you see where stuff breaks.
Sure. Let's tweak them once initial thing works fine.
Thanks!!
Madhav
>
> BR
> Jani.
>
>
> >
> > Regards,
> > Madhav
> >
> >> static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
> >> const struct intel_crtc_state *pipe_config,
> >> const struct drm_connector_state
> >> *conn_state) {
> >> + struct drm_crtc *crtc = pipe_config->base.crtc;
> >> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >> uint8_t mask = pipe_config->lane_lat_optim_mask;
> >>
> >> bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
> >> +
> >> + intel_enable_shared_dpll(intel_crtc);
> >> }
> >>
> >> void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp) @@
> >> -2730,6
> >> +2744,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv,
> >> +enum
> >> port port)
> >> intel_encoder->enable = intel_enable_ddi;
> >> if (IS_GEN9_LP(dev_priv))
> >> intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> >> + else
> >> + intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
> >> intel_encoder->pre_enable = intel_ddi_pre_enable;
> >> intel_encoder->disable = intel_disable_ddi;
> >> intel_encoder->post_disable = intel_ddi_post_disable; diff --git
> >> a/drivers/gpu/drm/i915/intel_display.c
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 6ed299670f27..d9ccde0a8097 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -5481,9 +5481,6 @@ static void haswell_crtc_enable(struct
> >> intel_crtc_state *pipe_config,
> >>
> >> intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
> >>
> >> - if (intel_crtc->config->shared_dpll)
> >> - intel_enable_shared_dpll(intel_crtc);
> >> -
> >> if (intel_crtc_has_dp_encoder(intel_crtc->config))
> >> intel_dp_set_m_n(intel_crtc, M1_N1);
> >>
> >> --
> >> 2.11.0
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] drm/i915: push shared dpll enable to encoders on DDI platforms
2017-12-18 13:05 ` Chauhan, Madhav
@ 2017-12-20 11:07 ` Chauhan, Madhav
0 siblings, 0 replies; 16+ messages in thread
From: Chauhan, Madhav @ 2017-12-20 11:07 UTC (permalink / raw)
To: Nikula, Jani, 'intel-gfx@lists.freedesktop.org'
Cc: 'Daniel Vetter'
> -----Original Message-----
> From: Chauhan, Madhav
> Sent: Monday, December 18, 2017 6:35 PM
> To: Nikula, Jani <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Subject: RE: [Intel-gfx] [PATCH 4/4] drm/i915: push shared dpll enable to
> encoders on DDI platforms
>
> > -----Original Message-----
> > From: Nikula, Jani
> > Sent: Monday, December 18, 2017 6:21 PM
> > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Subject: RE: [Intel-gfx] [PATCH 4/4] drm/i915: push shared dpll enable
> > to encoders on DDI platforms
> >
> > On Mon, 18 Dec 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com>
> > wrote:
> > >> -----Original Message-----
> > >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> > >> Behalf Of Jani Nikula
> > >> Sent: Thursday, October 12, 2017 9:36 PM
> > >> To: intel-gfx@lists.freedesktop.org
> > >> Cc: Nikula, Jani <jani.nikula@intel.com>; Daniel Vetter
> > >> <daniel.vetter@ffwll.ch>
> > >> Subject: [Intel-gfx] [PATCH 4/4] drm/i915: push shared dpll enable
> > >> to encoders on DDI platforms
> > >>
> > >> As we move more encoder specific stuff to encoders on DDI
> > >> platforms, follow suit with shared dpll enable. In the future,
> > >> we'll need this refactoring for further encoder specific details in
> > >> the middle of the enable
> > sequence.
> > >>
> > >> v2: drop ifs around the intel_enable_shared_dpll calls (Daniel)
> > >>
> > >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > >> ---
> > >> drivers/gpu/drm/i915/intel_crt.c | 2 ++
> > >> drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++++++++++++
> > >> drivers/gpu/drm/i915/intel_display.c | 3 ---
> > >> 3 files changed, 18 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_crt.c
> > >> b/drivers/gpu/drm/i915/intel_crt.c
> > >> index 67f771f24608..65f28062cb51 100644
> > >> --- a/drivers/gpu/drm/i915/intel_crt.c
> > >> +++ b/drivers/gpu/drm/i915/intel_crt.c
> > >> @@ -255,6 +255,8 @@ static void hsw_pre_pll_enable_crt(struct
> > >> intel_encoder *encoder,
> > >> WARN_ON(!intel_crtc->config->has_pch_encoder);
> > >>
> > >> intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > >> +
> > >> + intel_enable_shared_dpll(intel_crtc);
> > >> }
> > >>
> > >> static void hsw_pre_enable_crt(struct intel_encoder *encoder, diff
> > >> --git a/drivers/gpu/drm/i915/intel_ddi.c
> > >> b/drivers/gpu/drm/i915/intel_ddi.c
> > >> index 9819e51fa160..05f9db9c3d52 100644
> > >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> > >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > >> @@ -2416,13 +2416,27 @@ static void intel_disable_ddi(struct
> > >> intel_encoder *intel_encoder,
> > >> }
> > >> }
> > >>
> > >> +static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> > >> + const struct intel_crtc_state *pipe_config,
> > >> + const struct drm_connector_state
> > >> *conn_state) {
> > >> + struct drm_crtc *crtc = pipe_config->base.crtc;
> > >> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > >> +
> > >> + intel_enable_shared_dpll(intel_crtc);
> > >> +}
> > >> +
> > >
> > > Shouldn’t we add this code (and below as well) inside enable() or
> > > pre_enable() functions of struct intel_encoder as pre_pll_enable()
> > > function
> > should have some code prior to enabling PLL.
> >
> > The first step should be pushing the calls down to encoder hooks while
> > keeping the modeset sequence as close as possible to before. After
> > that, we can start tweaking the order where applicable. Always make
> > minimal changes so you see where stuff breaks.
>
> Sure. Let's tweak them once initial thing works fine.
> Thanks!!
LGTM with above clarification.
Reviewed-by: Madhav Chauhan <madhav.chauhan@intel.com>
Regards,
Madhav
>
> Madhav
>
> >
> > BR
> > Jani.
> >
> >
> > >
> > > Regards,
> > > Madhav
> > >
> > >> static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
> > >> const struct intel_crtc_state *pipe_config,
> > >> const struct drm_connector_state
> > >> *conn_state) {
> > >> + struct drm_crtc *crtc = pipe_config->base.crtc;
> > >> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > >> uint8_t mask = pipe_config->lane_lat_optim_mask;
> > >>
> > >> bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
> > >> +
> > >> + intel_enable_shared_dpll(intel_crtc);
> > >> }
> > >>
> > >> void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp) @@
> > >> -2730,6
> > >> +2744,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv,
> > >> +enum
> > >> port port)
> > >> intel_encoder->enable = intel_enable_ddi;
> > >> if (IS_GEN9_LP(dev_priv))
> > >> intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> > >> + else
> > >> + intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
> > >> intel_encoder->pre_enable = intel_ddi_pre_enable;
> > >> intel_encoder->disable = intel_disable_ddi;
> > >> intel_encoder->post_disable = intel_ddi_post_disable; diff --git
> > >> a/drivers/gpu/drm/i915/intel_display.c
> > >> b/drivers/gpu/drm/i915/intel_display.c
> > >> index 6ed299670f27..d9ccde0a8097 100644
> > >> --- a/drivers/gpu/drm/i915/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/intel_display.c
> > >> @@ -5481,9 +5481,6 @@ static void haswell_crtc_enable(struct
> > >> intel_crtc_state *pipe_config,
> > >>
> > >> intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
> > >>
> > >> - if (intel_crtc->config->shared_dpll)
> > >> - intel_enable_shared_dpll(intel_crtc);
> > >> -
> > >> if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > >> intel_dp_set_m_n(intel_crtc, M1_N1);
> > >>
> > >> --
> > >> 2.11.0
> > >>
> > >> _______________________________________________
> > >> Intel-gfx mailing list
> > >> Intel-gfx@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI
2017-10-12 16:05 [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI Jani Nikula
` (2 preceding siblings ...)
2017-10-12 16:05 ` [PATCH 4/4] drm/i915: push shared dpll enable to encoders on DDI platforms Jani Nikula
@ 2017-10-12 16:17 ` Ville Syrjälä
2017-10-12 16:30 ` ✗ Fi.CI.BAT: failure for series starting with [1/4] " Patchwork
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2017-10-12 16:17 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx
On Thu, Oct 12, 2017 at 07:05:30PM +0300, Jani Nikula wrote:
> This is never set to true on DDI platforms, so there's no need to set it
> to false either.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 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 7e91dc9a0fcf..2faa938f57a3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9025,8 +9025,6 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> }
> }
>
> - crtc->lowfreq_avail = false;
> -
IMO just kill off this thing entirely. It's a g4x only thing, and I'm
not sure it even works there. I have a branch somewhere where I extend
our drrs support to cover all platform (g4x included), but I'd need
to find some extra time to polish the few remaining rough edges.
In the meantime this thing (assuming it does something) might just
cause random vblank timing errors and whanot, and we don't want that.
> return 0;
> }
>
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: remove redundant lowfreq_avail setting for DDI
2017-10-12 16:05 [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI Jani Nikula
` (3 preceding siblings ...)
2017-10-12 16:17 ` [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI Ville Syrjälä
@ 2017-10-12 16:30 ` Patchwork
2017-10-12 16:40 ` [PATCH v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr Jani Nikula
2017-10-12 17:19 ` ✗ Fi.CI.BAT: failure for series starting with [v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr (rev2) Patchwork
6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2017-10-12 16:30 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: remove redundant lowfreq_avail setting for DDI
URL : https://patchwork.freedesktop.org/series/31832/
State : failure
== Summary ==
Series 31832v1 series starting with [1/4] drm/i915: remove redundant lowfreq_avail setting for DDI
https://patchwork.freedesktop.org/api/1.0/series/31832/revisions/1/mbox/
Test core_auth:
Subgroup basic-auth:
pass -> DMESG-WARN (fi-cnl-y)
Test core_prop_blob:
Subgroup basic:
pass -> DMESG-WARN (fi-cnl-y)
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-cnl-y)
Test gem_exec_reloc:
Subgroup basic-cpu-gtt-active:
fail -> PASS (fi-gdg-551) fdo#102582 +3
Subgroup basic-write-cpu-active:
pass -> FAIL (fi-gdg-551)
Subgroup basic-write-gtt-active:
pass -> FAIL (fi-gdg-551)
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-cfl-s)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass -> DMESG-WARN (fi-hsw-4770r)
pass -> DMESG-WARN (fi-bdw-5557u)
pass -> DMESG-WARN (fi-bdw-gvtdvm)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k) fdo#100367 +2
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-kbl-7567u) fdo#102846 +5
pass -> DMESG-WARN (fi-glk-1)
pass -> DMESG-WARN (fi-cfl-s)
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-hsw-4770r)
pass -> DMESG-WARN (fi-bdw-5557u)
pass -> DMESG-WARN (fi-bdw-gvtdvm)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6770hq) fdo#102990
pass -> DMESG-WARN (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-glk-1)
pass -> DMESG-WARN (fi-cfl-s) fdo#102673
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-WARN (fi-hsw-4770r)
pass -> DMESG-WARN (fi-bdw-5557u)
pass -> DMESG-WARN (fi-bdw-gvtdvm)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-glk-1)
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> DMESG-WARN (fi-bxt-j4205)
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367
fdo#102846 https://bugs.freedesktop.org/show_bug.cgi?id=102846
fdo#102990 https://bugs.freedesktop.org/show_bug.cgi?id=102990
fdo#102673 https://bugs.freedesktop.org/show_bug.cgi?id=102673
fi-bdw-5557u total:289 pass:265 dwarn:3 dfail:0 fail:0 skip:21 time:457s
fi-bdw-gvtdvm total:289 pass:262 dwarn:3 dfail:0 fail:0 skip:24 time:473s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:391s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:584s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:288s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:525s
fi-bxt-j4205 total:289 pass:256 dwarn:4 dfail:0 fail:0 skip:29 time:523s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:541s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:527s
fi-cfl-s total:289 pass:250 dwarn:7 dfail:0 fail:0 skip:32 time:564s
fi-cnl-y total:12 pass:0 dwarn:2 dfail:0 fail:0 skip:9
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:433s
fi-gdg-551 total:289 pass:173 dwarn:1 dfail:0 fail:6 skip:109 time:274s
fi-glk-1 total:289 pass:258 dwarn:3 dfail:0 fail:0 skip:28 time:605s
fi-hsw-4770r total:289 pass:259 dwarn:3 dfail:0 fail:0 skip:27 time:440s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:472s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:507s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:482s
fi-kbl-7500u total:289 pass:261 dwarn:4 dfail:0 fail:0 skip:24 time:511s
fi-kbl-7567u total:289 pass:262 dwarn:7 dfail:0 fail:0 skip:20 time:497s
fi-kbl-r total:289 pass:259 dwarn:3 dfail:0 fail:0 skip:27 time:600s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:667s
fi-skl-6260u total:289 pass:266 dwarn:3 dfail:0 fail:0 skip:20 time:478s
fi-skl-6700hq total:289 pass:260 dwarn:3 dfail:0 fail:0 skip:26 time:663s
fi-skl-6700k total:289 pass:262 dwarn:3 dfail:0 fail:0 skip:24 time:531s
fi-skl-6770hq total:289 pass:266 dwarn:3 dfail:0 fail:0 skip:20 time:512s
fi-skl-gvtdvm total:289 pass:263 dwarn:3 dfail:0 fail:0 skip:23 time:476s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:591s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:437s
e8024950de9e2f104a51384a16dae9a08388d43f drm-tip: 2017y-10m-12d-15h-17m-52s UTC integration manifest
00601a8e6aa3 drm/i915: push shared dpll enable to encoders on DDI platforms
d3068236b5a1 drm/i915: push crtc compute clock to encoder compute config on DDI
fbea6b19f176 drm/i915/crt: split compute_config hook by platforms
f9c2da2c340d drm/i915: remove redundant lowfreq_avail setting for DDI
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6009/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr
2017-10-12 16:05 [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI Jani Nikula
` (4 preceding siblings ...)
2017-10-12 16:30 ` ✗ Fi.CI.BAT: failure for series starting with [1/4] " Patchwork
@ 2017-10-12 16:40 ` Jani Nikula
2017-10-12 16:43 ` Ville Syrjälä
2017-10-12 17:19 ` ✗ Fi.CI.BAT: failure for series starting with [v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr (rev2) Patchwork
6 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2017-10-12 16:40 UTC (permalink / raw)
To: Jani Nikula, intel-gfx; +Cc: Daniel Vetter
They're unused and unsupported. Leave the reduced_clock pointers in
place still, should they prove useful later on.
v2: go from nuking DDI lowfrew_avail to nuking it entirely (Ville)
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 --
drivers/gpu/drm/i915/i915_pci.c | 2 --
drivers/gpu/drm/i915/intel_display.c | 15 ---------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
4 files changed, 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6bbc4b83aa0a..e655b91944b7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -785,7 +785,6 @@ struct intel_csr {
func(has_logical_ring_contexts); \
func(has_logical_ring_preemption); \
func(has_overlay); \
- func(has_pipe_cxsr); \
func(has_pooled_eu); \
func(has_psr); \
func(has_rc6); \
@@ -3168,7 +3167,6 @@ intel_info(const struct drm_i915_private *dev_priv)
#define I915_HAS_HOTPLUG(dev_priv) ((dev_priv)->info.has_hotplug)
#define HAS_FW_BLC(dev_priv) (INTEL_GEN(dev_priv) > 2)
-#define HAS_PIPE_CXSR(dev_priv) ((dev_priv)->info.has_pipe_cxsr)
#define HAS_FBC(dev_priv) ((dev_priv)->info.has_fbc)
#define HAS_CUR_FBC(dev_priv) (!HAS_GMCH_DISPLAY(dev_priv) && INTEL_INFO(dev_priv)->gen >= 7)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index bf467f30c99b..c162477ad1ff 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -193,7 +193,6 @@ static const struct intel_device_info intel_i965gm_info __initconst = {
static const struct intel_device_info intel_g45_info __initconst = {
GEN4_FEATURES,
.platform = INTEL_G45,
- .has_pipe_cxsr = 1,
.ring_mask = RENDER_RING | BSD_RING,
};
@@ -201,7 +200,6 @@ static const struct intel_device_info intel_gm45_info __initconst = {
GEN4_FEATURES,
.platform = INTEL_GM45,
.is_mobile = 1, .has_fbc = 1,
- .has_pipe_cxsr = 1,
.supports_tv = 1,
.ring_mask = RENDER_RING | BSD_RING,
};
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7e91dc9a0fcf..8e5bcb7c1136 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6522,11 +6522,9 @@ static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
crtc_state->dpll_hw_state.fp0 = fp;
- crtc->lowfreq_avail = false;
if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
reduced_clock) {
crtc_state->dpll_hw_state.fp1 = fp2;
- crtc->lowfreq_avail = true;
} else {
crtc_state->dpll_hw_state.fp1 = fp;
}
@@ -7221,15 +7219,6 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
}
}
- if (HAS_PIPE_CXSR(dev_priv)) {
- if (intel_crtc->lowfreq_avail) {
- DRM_DEBUG_KMS("enabling CxSR downclocking\n");
- pipeconf |= PIPECONF_CXSR_DOWNCLOCK;
- } else {
- DRM_DEBUG_KMS("disabling CxSR downclocking\n");
- }
- }
-
if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
if (INTEL_GEN(dev_priv) < 4 ||
intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
@@ -8365,8 +8354,6 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
memset(&crtc_state->dpll_hw_state, 0,
sizeof(crtc_state->dpll_hw_state));
- crtc->lowfreq_avail = false;
-
/* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
if (!crtc_state->has_pch_encoder)
return 0;
@@ -9025,8 +9012,6 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
}
}
- crtc->lowfreq_avail = false;
-
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b87946dcc53f..5b7db672a193 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -800,7 +800,6 @@ struct intel_crtc {
* some outputs connected to this crtc.
*/
bool active;
- bool lowfreq_avail;
u8 plane_ids_mask;
unsigned long long enabled_power_domains;
struct intel_overlay *overlay;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr
2017-10-12 16:40 ` [PATCH v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr Jani Nikula
@ 2017-10-12 16:43 ` Ville Syrjälä
0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2017-10-12 16:43 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx
On Thu, Oct 12, 2017 at 07:40:32PM +0300, Jani Nikula wrote:
> They're unused and unsupported. Leave the reduced_clock pointers in
> place still, should they prove useful later on.
>
> v2: go from nuking DDI lowfrew_avail to nuking it entirely (Ville)
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 --
> drivers/gpu/drm/i915/i915_pci.c | 2 --
> drivers/gpu/drm/i915/intel_display.c | 15 ---------------
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> 4 files changed, 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6bbc4b83aa0a..e655b91944b7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -785,7 +785,6 @@ struct intel_csr {
> func(has_logical_ring_contexts); \
> func(has_logical_ring_preemption); \
> func(has_overlay); \
> - func(has_pipe_cxsr); \
> func(has_pooled_eu); \
> func(has_psr); \
> func(has_rc6); \
> @@ -3168,7 +3167,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> #define I915_HAS_HOTPLUG(dev_priv) ((dev_priv)->info.has_hotplug)
>
> #define HAS_FW_BLC(dev_priv) (INTEL_GEN(dev_priv) > 2)
> -#define HAS_PIPE_CXSR(dev_priv) ((dev_priv)->info.has_pipe_cxsr)
> #define HAS_FBC(dev_priv) ((dev_priv)->info.has_fbc)
> #define HAS_CUR_FBC(dev_priv) (!HAS_GMCH_DISPLAY(dev_priv) && INTEL_INFO(dev_priv)->gen >= 7)
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index bf467f30c99b..c162477ad1ff 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -193,7 +193,6 @@ static const struct intel_device_info intel_i965gm_info __initconst = {
> static const struct intel_device_info intel_g45_info __initconst = {
> GEN4_FEATURES,
> .platform = INTEL_G45,
> - .has_pipe_cxsr = 1,
> .ring_mask = RENDER_RING | BSD_RING,
> };
>
> @@ -201,7 +200,6 @@ static const struct intel_device_info intel_gm45_info __initconst = {
> GEN4_FEATURES,
> .platform = INTEL_GM45,
> .is_mobile = 1, .has_fbc = 1,
> - .has_pipe_cxsr = 1,
> .supports_tv = 1,
> .ring_mask = RENDER_RING | BSD_RING,
> };
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7e91dc9a0fcf..8e5bcb7c1136 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6522,11 +6522,9 @@ static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
>
> crtc_state->dpll_hw_state.fp0 = fp;
>
> - crtc->lowfreq_avail = false;
> if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> reduced_clock) {
> crtc_state->dpll_hw_state.fp1 = fp2;
> - crtc->lowfreq_avail = true;
> } else {
> crtc_state->dpll_hw_state.fp1 = fp;
> }
> @@ -7221,15 +7219,6 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
> }
> }
>
> - if (HAS_PIPE_CXSR(dev_priv)) {
> - if (intel_crtc->lowfreq_avail) {
> - DRM_DEBUG_KMS("enabling CxSR downclocking\n");
> - pipeconf |= PIPECONF_CXSR_DOWNCLOCK;
> - } else {
> - DRM_DEBUG_KMS("disabling CxSR downclocking\n");
> - }
> - }
> -
> if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
> if (INTEL_GEN(dev_priv) < 4 ||
> intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
> @@ -8365,8 +8354,6 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
> memset(&crtc_state->dpll_hw_state, 0,
> sizeof(crtc_state->dpll_hw_state));
>
> - crtc->lowfreq_avail = false;
> -
> /* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
> if (!crtc_state->has_pch_encoder)
> return 0;
> @@ -9025,8 +9012,6 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> }
> }
>
> - crtc->lowfreq_avail = false;
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b87946dcc53f..5b7db672a193 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -800,7 +800,6 @@ struct intel_crtc {
> * some outputs connected to this crtc.
> */
> bool active;
> - bool lowfreq_avail;
> u8 plane_ids_mask;
> unsigned long long enabled_power_domains;
> struct intel_overlay *overlay;
> --
> 2.11.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr (rev2)
2017-10-12 16:05 [PATCH 1/4] drm/i915: remove redundant lowfreq_avail setting for DDI Jani Nikula
` (5 preceding siblings ...)
2017-10-12 16:40 ` [PATCH v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr Jani Nikula
@ 2017-10-12 17:19 ` Patchwork
6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2017-10-12 17:19 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr (rev2)
URL : https://patchwork.freedesktop.org/series/31832/
State : failure
== Summary ==
Series 31832v2 series starting with [v2] drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr
https://patchwork.freedesktop.org/api/1.0/series/31832/revisions/2/mbox/
Test core_auth:
Subgroup basic-auth:
pass -> DMESG-WARN (fi-cnl-y)
Test core_prop_blob:
Subgroup basic:
pass -> DMESG-WARN (fi-cnl-y)
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-cnl-y)
Test gem_exec_reloc:
Subgroup basic-cpu-gtt-active:
fail -> PASS (fi-gdg-551) fdo#102582
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-cfl-s)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass -> DMESG-WARN (fi-hsw-4770r)
pass -> DMESG-WARN (fi-bdw-5557u)
pass -> DMESG-WARN (fi-bdw-gvtdvm)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k) fdo#100367 +2
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-kbl-7567u) fdo#102846 +5
pass -> DMESG-WARN (fi-glk-1)
pass -> DMESG-WARN (fi-cfl-s)
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-hsw-4770r)
pass -> DMESG-WARN (fi-bdw-5557u)
pass -> DMESG-WARN (fi-bdw-gvtdvm)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6770hq) fdo#102990
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-glk-1)
pass -> DMESG-WARN (fi-cfl-s) fdo#102673
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-WARN (fi-hsw-4770r)
pass -> DMESG-WARN (fi-bdw-5557u)
pass -> DMESG-WARN (fi-bdw-gvtdvm)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-glk-1)
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> DMESG-WARN (fi-bxt-j4205)
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367
fdo#102846 https://bugs.freedesktop.org/show_bug.cgi?id=102846
fdo#102990 https://bugs.freedesktop.org/show_bug.cgi?id=102990
fdo#102673 https://bugs.freedesktop.org/show_bug.cgi?id=102673
fi-bdw-5557u total:289 pass:265 dwarn:3 dfail:0 fail:0 skip:21 time:456s
fi-bdw-gvtdvm total:289 pass:262 dwarn:3 dfail:0 fail:0 skip:24 time:470s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:390s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:567s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:287s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:523s
fi-bxt-j4205 total:289 pass:256 dwarn:4 dfail:0 fail:0 skip:29 time:515s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:536s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:521s
fi-cfl-s total:289 pass:250 dwarn:7 dfail:0 fail:0 skip:32 time:560s
fi-cnl-y total:12 pass:0 dwarn:2 dfail:0 fail:0 skip:9
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:437s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:272s
fi-glk-1 total:289 pass:258 dwarn:3 dfail:0 fail:0 skip:28 time:608s
fi-hsw-4770r total:289 pass:259 dwarn:3 dfail:0 fail:0 skip:27 time:444s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:467s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:503s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:473s
fi-kbl-7500u total:289 pass:261 dwarn:4 dfail:0 fail:0 skip:24 time:507s
fi-kbl-7567u total:289 pass:262 dwarn:7 dfail:0 fail:0 skip:20 time:489s
fi-kbl-r total:289 pass:259 dwarn:3 dfail:0 fail:0 skip:27 time:595s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:660s
fi-skl-6260u total:289 pass:266 dwarn:3 dfail:0 fail:0 skip:20 time:462s
fi-skl-6700hq total:289 pass:260 dwarn:3 dfail:0 fail:0 skip:26 time:662s
fi-skl-6700k total:289 pass:262 dwarn:3 dfail:0 fail:0 skip:24 time:528s
fi-skl-6770hq total:289 pass:266 dwarn:3 dfail:0 fail:0 skip:20 time:504s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:583s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:427s
fi-skl-gvtdvm failed to connect after reboot
e8024950de9e2f104a51384a16dae9a08388d43f drm-tip: 2017y-10m-12d-15h-17m-52s UTC integration manifest
f12cd8334fc6 drm/i915: push shared dpll enable to encoders on DDI platforms
caf286aebd1f drm/i915: push crtc compute clock to encoder compute config on DDI
b236d98d8ed0 drm/i915/crt: split compute_config hook by platforms
6fbab23f6523 drm/i915: remove g4x lowfrew_avail and has_pipe_cxsr
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6010/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread