[PATCH v2 1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)

[PATCH v2 1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)

[Hans de Goede]

When we are doing a fastset (needs_modeset=false, update_pipe=true) we
may need to update some encoder-level things such as checking that PSR
is enabled.

This commit adds an update_pipe callback to intel_encoder and a new
intel_encoders_update_pipe helper which calls this for all encoders
connected to a crtc. The new intel_encoders_update_pipe helper is called
from intel_update_crtc when doing a fastset.

Changes in v2:
-Name the new encoder callback update_pipe instead of just update

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  3 +++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a858238972a2..2876a3752079 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5578,6 +5578,26 @@ static void intel_encoders_post_pll_disable(struct drm_crtc *crtc,
 	}
 }
 
+static void intel_encoders_update_pipe(struct drm_crtc *crtc,
+				       struct intel_crtc_state *crtc_state,
+				       struct drm_atomic_state *old_state)
+{
+	struct drm_connector_state *conn_state;
+	struct drm_connector *conn;
+	int i;
+
+	for_each_new_connector_in_state(old_state, conn, conn_state, i) {
+		struct intel_encoder *encoder =
+			to_intel_encoder(conn_state->best_encoder);
+
+		if (conn_state->crtc != crtc)
+			continue;
+
+		if (encoder->update_pipe)
+			encoder->update_pipe(encoder, crtc_state, conn_state);
+	}
+}
+
 static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 				 struct drm_atomic_state *old_state)
 {
@@ -12750,6 +12770,9 @@ static void intel_update_crtc(struct drm_crtc *crtc,
 	} else {
 		intel_pre_plane_update(to_intel_crtc_state(old_crtc_state),
 				       pipe_config);
+
+		if (pipe_config->update_pipe)
+			intel_encoders_update_pipe(crtc, pipe_config, state);
 	}
 
 	if (new_plane_state)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ef1315d7c4ae..8717b873cd53 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -243,6 +243,9 @@ struct intel_encoder {
 	void (*post_pll_disable)(struct intel_encoder *,
 				 const struct intel_crtc_state *,
 				 const struct drm_connector_state *);
+	void (*update_pipe)(struct intel_encoder *,
+			    const struct intel_crtc_state *,
+			    const struct drm_connector_state *);
 	/* Read out the current hw state of this connector, returning true if
 	 * the encoder is active. If the encoder is enabled it also set the pipe
 	 * it is connected to in the pipe parameter. */
-- 
2.20.1

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

[PATCH v2 2/3] drm/i915: Allow calling intel_edp_drrs_enable twice

[Hans de Goede]

Do not make it an error to call intel_edp_drrs_enable while drrs has
already been enabled, instead exit silently in this case.

This is a preparation patch for ensuring that DRRS is enabled on fastsets.

Note that the removed WARN_ON could also be triggered from userspace
through the i915_drrs_ctl debugfs entry which was added by
commit 35954e88bc50 ("drm/i915: Runtime disable for eDP DRRS")

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5b601b754707..62fd11540942 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6432,8 +6432,8 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,
 	}
 
 	mutex_lock(&dev_priv->drrs.mutex);
-	if (WARN_ON(dev_priv->drrs.dp)) {
-		DRM_ERROR("DRRS already enabled\n");
+	if (dev_priv->drrs.dp) {
+		DRM_DEBUG_KMS("DRRS already enabled\n");
 		goto unlock;
 	}
 
-- 
2.20.1

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

[PATCH v2 3/3] drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)

[Hans de Goede]

Call intel_psr_enable() and intel_edp_drrs_enable() on pipe updates to make
sure that we enable PSR / DRRS (when applicable) on fastsets.

Note calling these functions when PSR / DRRS has already been enabled is a
no-op, so it is safe to do this on every encoder->update_pipe callback.

Changes in v2:
-Merge the patches adding the intel_psr_enable() and intel_edp_drrs_enable()
 calls into a single patch

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e3cc19e19199..fdf57f451b72 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3537,6 +3537,24 @@ static void intel_disable_ddi(struct intel_encoder *encoder,
 		intel_disable_ddi_dp(encoder, old_crtc_state, old_conn_state);
 }
 
+static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
+				     const struct intel_crtc_state *crtc_state,
+				     const struct drm_connector_state *conn_state)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	intel_psr_enable(intel_dp, crtc_state);
+	intel_edp_drrs_enable(intel_dp, crtc_state);
+}
+
+static void intel_ddi_update_pipe(struct intel_encoder *encoder,
+				  const struct intel_crtc_state *crtc_state,
+				  const struct drm_connector_state *conn_state)
+{
+	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
+		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
+}
+
 static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
 					 const struct intel_crtc_state *pipe_config,
 					 enum port port)
@@ -4169,6 +4187,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;
 	intel_encoder->post_disable = intel_ddi_post_disable;
+	intel_encoder->update_pipe = intel_ddi_update_pipe;
 	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
 	intel_encoder->get_config = intel_ddi_get_config;
 	intel_encoder->suspend = intel_ddi_encoder_suspend;
-- 
2.20.1

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

Re: [PATCH v2 3/3] drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)

[Rodrigo Vivi]

On Thu, Dec 20, 2018 at 02:21:20PM +0100, Hans de Goede wrote:
> Call intel_psr_enable() and intel_edp_drrs_enable() on pipe updates to make
> sure that we enable PSR / DRRS (when applicable) on fastsets.
> 
> Note calling these functions when PSR / DRRS has already been enabled is a
> no-op, so it is safe to do this on every encoder->update_pipe callback.
> 
> Changes in v2:
> -Merge the patches adding the intel_psr_enable() and intel_edp_drrs_enable()
>  calls into a single patch
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e3cc19e19199..fdf57f451b72 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3537,6 +3537,24 @@ static void intel_disable_ddi(struct intel_encoder *encoder,
>  		intel_disable_ddi_dp(encoder, old_crtc_state, old_conn_state);
>  }
>  
> +static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
> +				     const struct intel_crtc_state *crtc_state,
> +				     const struct drm_connector_state *conn_state)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +	intel_psr_enable(intel_dp, crtc_state);
> +	intel_edp_drrs_enable(intel_dp, crtc_state);
> +}
> +
> +static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> +				  const struct intel_crtc_state *crtc_state,
> +				  const struct drm_connector_state *conn_state)
> +{
> +	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> +		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> +}
> +
>  static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
>  					 const struct intel_crtc_state *pipe_config,
>  					 enum port port)
> @@ -4169,6 +4187,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	intel_encoder->pre_enable = intel_ddi_pre_enable;
>  	intel_encoder->disable = intel_disable_ddi;
>  	intel_encoder->post_disable = intel_ddi_post_disable;
> +	intel_encoder->update_pipe = intel_ddi_update_pipe;
>  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
>  	intel_encoder->get_config = intel_ddi_get_config;
>  	intel_encoder->suspend = intel_ddi_encoder_suspend;
> -- 
> 2.20.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)

[Dhinakaran Pandiyan]

On Thu, 2018-12-20 at 09:10 -0800, Rodrigo Vivi wrote:
> On Thu, Dec 20, 2018 at 02:21:20PM +0100, Hans de Goede wrote:
> > Call intel_psr_enable() and intel_edp_drrs_enable() on pipe updates
> > to make
> > sure that we enable PSR / DRRS (when applicable) on fastsets.

I am probably missing something, doesn't intel_pipe_config_compare()
need to check for pipe_config->has_psr? And also read the hardware PSR
state at boot?

> > 
> > Note calling these functions when PSR / DRRS has already been
> > enabled is a
> > no-op, so it is safe to do this on every encoder->update_pipe
> > callback.
> > 
> > Changes in v2:
> > -Merge the patches adding the intel_psr_enable() and
> > intel_edp_drrs_enable()
> >  calls into a single patch
> > 
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> 
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index e3cc19e19199..fdf57f451b72 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3537,6 +3537,24 @@ static void intel_disable_ddi(struct
> > intel_encoder *encoder,
> >  		intel_disable_ddi_dp(encoder, old_crtc_state,
> > old_conn_state);
> >  }
> >  
> > +static void intel_ddi_update_pipe_dp(struct intel_encoder
> > *encoder,
> > +				     const struct intel_crtc_state
> > *crtc_state,
> > +				     const struct drm_connector_state
> > *conn_state)
> > +{
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +
> > +	intel_psr_enable(intel_dp, crtc_state);
> > +	intel_edp_drrs_enable(intel_dp, crtc_state);
> > +}
> > +
> > +static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> > +				  const struct intel_crtc_state
> > *crtc_state,
> > +				  const struct drm_connector_state
> > *conn_state)
> > +{
> > +	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))

We could restrict this to eDP outputs as both PSR and DRRS are eDP
features.

> > +		intel_ddi_update_pipe_dp(encoder, crtc_state,
> > conn_state);
> > +}
> > +
> >  static void intel_ddi_set_fia_lane_count(struct intel_encoder
> > *encoder,
> >  					 const struct intel_crtc_state
> > *pipe_config,
> >  					 enum port port)
> > @@ -4169,6 +4187,7 @@ void intel_ddi_init(struct drm_i915_private
> > *dev_priv, enum port port)
> >  	intel_encoder->pre_enable = intel_ddi_pre_enable;
> >  	intel_encoder->disable = intel_disable_ddi;
> >  	intel_encoder->post_disable = intel_ddi_post_disable;
> > +	intel_encoder->update_pipe = intel_ddi_update_pipe;
> >  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
> >  	intel_encoder->get_config = intel_ddi_get_config;
> >  	intel_encoder->suspend = intel_ddi_encoder_suspend;
> > -- 
> > 2.20.1
> > 

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

Re: [PATCH v2 3/3] drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)

[Dhinakaran Pandiyan]

On Thu, 2018-12-20 at 15:13 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2018-12-20 at 09:10 -0800, Rodrigo Vivi wrote:
> > On Thu, Dec 20, 2018 at 02:21:20PM +0100, Hans de Goede wrote:
> > > Call intel_psr_enable() and intel_edp_drrs_enable() on pipe
> > > updates
> > > to make
> > > sure that we enable PSR / DRRS (when applicable) on fastsets.
> 
> I am probably missing something, doesn't intel_pipe_config_compare()
> need to check for pipe_config->has_psr? And also read the hardware
> PSR
> state at boot?
Answering my own question here, pipe_config_compare() returns true
lacking a comparison for ->has_psr. And we assume the bios does not
enable PSR, so no need to read the hardware state.
> > > 
> > > Note calling these functions when PSR / DRRS has already been
> > > enabled is a
> > > no-op, so it is safe to do this on every encoder->update_pipe
> > > callback.
> > > 
> > > Changes in v2:
> > > -Merge the patches adding the intel_psr_enable() and
> > > intel_edp_drrs_enable()
> > >  calls into a single patch
> > > 
> > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com
> > > >
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > 
> > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 19 +++++++++++++++++++
> > >  1 file changed, 19 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index e3cc19e19199..fdf57f451b72 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -3537,6 +3537,24 @@ static void intel_disable_ddi(struct
> > > intel_encoder *encoder,
> > >  		intel_disable_ddi_dp(encoder, old_crtc_state,
> > > old_conn_state);
> > >  }
> > >  
> > > +static void intel_ddi_update_pipe_dp(struct intel_encoder
> > > *encoder,
> > > +				     const struct intel_crtc_state
> > > *crtc_state,
> > > +				     const struct drm_connector_state
> > > *conn_state)
> > > +{
> > > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > > +
> > > +	intel_psr_enable(intel_dp, crtc_state);
> > > +	intel_edp_drrs_enable(intel_dp, crtc_state);
> > > +}
> > > +
> > > +static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> > > +				  const struct intel_crtc_state
> > > *crtc_state,
> > > +				  const struct drm_connector_state
> > > *conn_state)
> > > +{
> > > +	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> 
> We could restrict this to eDP outputs as both PSR and DRRS are eDP
> features.
> 
> > > +		intel_ddi_update_pipe_dp(encoder, crtc_state,
> > > conn_state);
> > > +}
> > > +
> > >  static void intel_ddi_set_fia_lane_count(struct intel_encoder
> > > *encoder,
> > >  					 const struct intel_crtc_state
> > > *pipe_config,
> > >  					 enum port port)
> > > @@ -4169,6 +4187,7 @@ void intel_ddi_init(struct drm_i915_private
> > > *dev_priv, enum port port)
> > >  	intel_encoder->pre_enable = intel_ddi_pre_enable;
> > >  	intel_encoder->disable = intel_disable_ddi;
> > >  	intel_encoder->post_disable = intel_ddi_post_disable;
> > > +	intel_encoder->update_pipe = intel_ddi_update_pipe;
> > >  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
> > >  	intel_encoder->get_config = intel_ddi_get_config;
> > >  	intel_encoder->suspend = intel_ddi_encoder_suspend;
> > > -- 
> > > 2.20.1
> > > 

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

Re: [PATCH v2 3/3] drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)

[Daniel Vetter]

On Fri, Dec 21, 2018 at 8:42 PM Dhinakaran Pandiyan
<dhinakaran.pandiyan@intel.com> wrote:
>
> On Thu, 2018-12-20 at 15:13 -0800, Dhinakaran Pandiyan wrote:
> > On Thu, 2018-12-20 at 09:10 -0800, Rodrigo Vivi wrote:
> > > On Thu, Dec 20, 2018 at 02:21:20PM +0100, Hans de Goede wrote:
> > > > Call intel_psr_enable() and intel_edp_drrs_enable() on pipe
> > > > updates
> > > > to make
> > > > sure that we enable PSR / DRRS (when applicable) on fastsets.
> >
> > I am probably missing something, doesn't intel_pipe_config_compare()
> > need to check for pipe_config->has_psr? And also read the hardware
> > PSR
> > state at boot?
> Answering my own question here, pipe_config_compare() returns true
> lacking a comparison for ->has_psr. And we assume the bios does not
> enable PSR, so no need to read the hardware state.

pipe_config_compare is also a validation tool, ensuring that all the
various fastboot (and other modeset) transitions work. Not reading out
& not comparing state reduces validation coverage. Some exceptions
apply, but generally they should be justified with other test
coverage, not with "it works without that". Everything is supposed to
work without the validation tools/tests :-)
-Daniel

> > > >
> > > > Note calling these functions when PSR / DRRS has already been
> > > > enabled is a
> > > > no-op, so it is safe to do this on every encoder->update_pipe
> > > > callback.
> > > >
> > > > Changes in v2:
> > > > -Merge the patches adding the intel_psr_enable() and
> > > > intel_edp_drrs_enable()
> > > >  calls into a single patch
> > > >
> > > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com
> > > > >
> > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > >
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > >
> > > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > >
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_ddi.c | 19 +++++++++++++++++++
> > > >  1 file changed, 19 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > > index e3cc19e19199..fdf57f451b72 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > @@ -3537,6 +3537,24 @@ static void intel_disable_ddi(struct
> > > > intel_encoder *encoder,
> > > >           intel_disable_ddi_dp(encoder, old_crtc_state,
> > > > old_conn_state);
> > > >  }
> > > >
> > > > +static void intel_ddi_update_pipe_dp(struct intel_encoder
> > > > *encoder,
> > > > +                              const struct intel_crtc_state
> > > > *crtc_state,
> > > > +                              const struct drm_connector_state
> > > > *conn_state)
> > > > +{
> > > > + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > > > +
> > > > + intel_psr_enable(intel_dp, crtc_state);
> > > > + intel_edp_drrs_enable(intel_dp, crtc_state);
> > > > +}
> > > > +
> > > > +static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> > > > +                           const struct intel_crtc_state
> > > > *crtc_state,
> > > > +                           const struct drm_connector_state
> > > > *conn_state)
> > > > +{
> > > > + if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> >
> > We could restrict this to eDP outputs as both PSR and DRRS are eDP
> > features.
> >
> > > > +         intel_ddi_update_pipe_dp(encoder, crtc_state,
> > > > conn_state);
> > > > +}
> > > > +
> > > >  static void intel_ddi_set_fia_lane_count(struct intel_encoder
> > > > *encoder,
> > > >                                    const struct intel_crtc_state
> > > > *pipe_config,
> > > >                                    enum port port)
> > > > @@ -4169,6 +4187,7 @@ void intel_ddi_init(struct drm_i915_private
> > > > *dev_priv, enum port port)
> > > >   intel_encoder->pre_enable = intel_ddi_pre_enable;
> > > >   intel_encoder->disable = intel_disable_ddi;
> > > >   intel_encoder->post_disable = intel_ddi_post_disable;
> > > > + intel_encoder->update_pipe = intel_ddi_update_pipe;
> > > >   intel_encoder->get_hw_state = intel_ddi_get_hw_state;
> > > >   intel_encoder->get_config = intel_ddi_get_config;
> > > >   intel_encoder->suspend = intel_ddi_encoder_suspend;
> > > > --
> > > > 2.20.1
> > > >
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)

[Hans de Goede]

Hi,

On 20-12-18 18:10, Rodrigo Vivi wrote:
> On Thu, Dec 20, 2018 at 02:21:20PM +0100, Hans de Goede wrote:
>> Call intel_psr_enable() and intel_edp_drrs_enable() on pipe updates to make
>> sure that we enable PSR / DRRS (when applicable) on fastsets.
>>
>> Note calling these functions when PSR / DRRS has already been enabled is a
>> no-op, so it is safe to do this on every encoder->update_pipe callback.
>>
>> Changes in v2:
>> -Merge the patches adding the intel_psr_enable() and intel_edp_drrs_enable()
>>   calls into a single patch
>>
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> 
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

After restoring my dinq mistake from yesterday I now have pushed this
properly.

Maarten, Rodrigo, thank you for the Review/Ack.

And with this wrapped up I'm to celebrate christmas with my family.

Merry christmas everyone.

Regards,

Hans




> 
>> ---
>>   drivers/gpu/drm/i915/intel_ddi.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index e3cc19e19199..fdf57f451b72 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -3537,6 +3537,24 @@ static void intel_disable_ddi(struct intel_encoder *encoder,
>>   		intel_disable_ddi_dp(encoder, old_crtc_state, old_conn_state);
>>   }
>>   
>> +static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
>> +				     const struct intel_crtc_state *crtc_state,
>> +				     const struct drm_connector_state *conn_state)
>> +{
>> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>> +
>> +	intel_psr_enable(intel_dp, crtc_state);
>> +	intel_edp_drrs_enable(intel_dp, crtc_state);
>> +}
>> +
>> +static void intel_ddi_update_pipe(struct intel_encoder *encoder,
>> +				  const struct intel_crtc_state *crtc_state,
>> +				  const struct drm_connector_state *conn_state)
>> +{
>> +	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>> +		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
>> +}
>> +
>>   static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
>>   					 const struct intel_crtc_state *pipe_config,
>>   					 enum port port)
>> @@ -4169,6 +4187,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>   	intel_encoder->pre_enable = intel_ddi_pre_enable;
>>   	intel_encoder->disable = intel_disable_ddi;
>>   	intel_encoder->post_disable = intel_ddi_post_disable;
>> +	intel_encoder->update_pipe = intel_ddi_update_pipe;
>>   	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
>>   	intel_encoder->get_config = intel_ddi_get_config;
>>   	intel_encoder->suspend = intel_ddi_encoder_suspend;
>> -- 
>> 2.20.1
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)
URL   : https://patchwork.freedesktop.org/series/54339/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
79612d0a1354 drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)
-:71: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct intel_encoder *' should also have an identifier name
#71: FILE: drivers/gpu/drm/i915/intel_drv.h:246:
+	void (*update_pipe)(struct intel_encoder *,

-:71: WARNING:FUNCTION_ARGUMENTS: function definition argument 'const struct intel_crtc_state *' should also have an identifier name
#71: FILE: drivers/gpu/drm/i915/intel_drv.h:246:
+	void (*update_pipe)(struct intel_encoder *,

-:71: WARNING:FUNCTION_ARGUMENTS: function definition argument 'const struct drm_connector_state *' should also have an identifier name
#71: FILE: drivers/gpu/drm/i915/intel_drv.h:246:
+	void (*update_pipe)(struct intel_encoder *,

total: 0 errors, 3 warnings, 0 checks, 44 lines checked
8b9d52c1738e drm/i915: Allow calling intel_edp_drrs_enable twice
d15d2e96832a drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)
-:14: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14: 
-Merge the patches adding the intel_psr_enable() and intel_edp_drrs_enable()

total: 0 errors, 1 warnings, 0 checks, 31 lines checked

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

✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)
URL   : https://patchwork.freedesktop.org/series/54339/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5336 -> Patchwork_11136
====================================================

Summary
-------

  **WARNING**

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

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

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

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

### IGT changes ###

#### Warnings ####

  * igt@amdgpu/amd_basic@query-info:
    - fi-bsw-kefka:       NOTRUN -> SKIP +55

  * igt@gem_mmap_gtt@basic-write-gtt:
    - fi-blb-e6850:       NOTRUN -> PASS +111

  * igt@kms_addfb_basic@invalid-set-prop-any:
    - fi-glk-dsi:         NOTRUN -> PASS +155

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       SKIP -> PASS +2

  * igt@kms_force_connector_basic@force-edid:
    - fi-glk-dsi:         NOTRUN -> SKIP +36

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
    - fi-blb-e6850:       NOTRUN -> SKIP +21

  * igt@prime_vgem@basic-fence-read:
    - fi-bsw-kefka:       NOTRUN -> PASS +232

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

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

### IGT changes ###

#### Issues hit ####

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

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

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       NOTRUN -> INCOMPLETE [fdo#107718]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-kefka:       INCOMPLETE [fdo#105876] / [fdo#108714] -> PASS

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

  * igt@gem_mmap_gtt@basic-small-copy-xy:
    - fi-glk-dsi:         INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (49 -> 43)
------------------------------

  Additional (1): fi-skl-6700hq 
  Missing    (7): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y 


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

    * Linux: CI_DRM_5336 -> Patchwork_11136

  CI_DRM_5336: 9fd194c1f8bd36cb623d842f5ed52b2ca2c83d18 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11136: d15d2e96832ae55868d3dbf5c9b33eaaccfa6597 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d15d2e96832a drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)
8b9d52c1738e drm/i915: Allow calling intel_edp_drrs_enable twice
79612d0a1354 drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)

== Logs ==

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

✓ Fi.CI.IGT: success for series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)
URL   : https://patchwork.freedesktop.org/series/54339/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5336_full -> Patchwork_11136_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-blt:
    - shard-skl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-skl:          PASS -> TIMEOUT [fdo#108039]

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

  * igt@i915_suspend@shrink:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#108784]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-c:
    - shard-glk:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232] +1

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

  * igt@kms_cursor_crc@cursor-256x256-sliding:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

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

  * igt@kms_flip_tiling@flip-to-x-tiled:
    - shard-iclb:         PASS -> FAIL [fdo#108134]

  * igt@kms_frontbuffer_tracking@fbcpsr-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#106978] / [fdo#107773]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +5

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-iclb:         PASS -> FAIL [fdo#103166] +2

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

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-glk:          PASS -> FAIL [fdo#103166] +2

  * igt@pm_rpm@gem-mmap-gtt:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#108840]

  
#### Possible fixes ####

  * igt@gem_eio@unwedge-stress:
    - shard-glk:          FAIL [fdo#105957] -> PASS

  * igt@gem_exec_fence@basic-busy-default:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          FAIL [fdo#108145] -> PASS +1

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

  * igt@kms_cursor_crc@cursor-128x42-offscreen:
    - shard-glk:          INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-glk:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-64x64-offscreen:
    - shard-skl:          FAIL [fdo#103232] -> PASS

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

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt:
    - shard-skl:          FAIL [fdo#105682] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-glk:          FAIL [fdo#103167] -> PASS +3

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-apl:          DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +12

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

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

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-iclb:         FAIL [fdo#103166] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-glk:          DMESG-FAIL [fdo#105763] / [fdo#106538] -> PASS

  * igt@pm_rpm@debugfs-read:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS

  * igt@pm_rpm@i2c:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  * igt@pm_rpm@sysfs-read:
    - shard-iclb:         INCOMPLETE [fdo#107713] / [fdo#108840] -> PASS

  * igt@pm_rpm@system-suspend-execbuf:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] / [fdo#107807] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - shard-iclb:         INCOMPLETE [fdo#108315] -> DMESG-FAIL [fdo#108569]

  * igt@i915_suspend@shrink:
    - shard-snb:          INCOMPLETE [fdo#105411] / [fdo#106886] -> DMESG-WARN [fdo#108784]

  
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#105957]: https://bugs.freedesktop.org/show_bug.cgi?id=105957
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108039]: https://bugs.freedesktop.org/show_bug.cgi?id=108039
  [fdo#108134]: https://bugs.freedesktop.org/show_bug.cgi?id=108134
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5336 -> Patchwork_11136

  CI_DRM_5336: 9fd194c1f8bd36cb623d842f5ed52b2ca2c83d18 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11136: d15d2e96832ae55868d3dbf5c9b33eaaccfa6597 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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