[PATCH 1/2] drm/i915: factor out intel_mode_target_cdclk

[PATCH 1/2] drm/i915: factor out intel_mode_target_cdclk

[Imre Deak]

For GEN9 the target cdclk frequency is needed during the modeset state check
phase too, so factor out this functionality.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2641053..9b68a5c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5899,21 +5899,37 @@ static int intel_mode_max_pixclk(struct drm_device *dev,
 	return max_pixclk;
 }
 
-static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
+static int intel_mode_target_cdclk(struct drm_i915_private *dev_priv,
+				   struct drm_atomic_state *state)
 {
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
-	int max_pixclk = intel_mode_max_pixclk(state->dev, state);
-	int cdclk, ret = 0;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv->dev, state);
+	int cdclk;
 
 	if (max_pixclk < 0)
 		return max_pixclk;
 
-	if (IS_VALLEYVIEW(dev_priv))
-		cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
-	else
+	if (IS_BROXTON(dev_priv)) {
 		cdclk = broxton_calc_cdclk(dev_priv, max_pixclk);
+	} else if (IS_VALLEYVIEW(dev_priv)) {
+		cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
+	} else {
+		MISSING_CASE(INTEL_INFO(dev_priv));
+		cdclk = 0;
+	}
+
+	return cdclk;
+}
+
+static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->dev);
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int cdclk, ret = 0;
+
+	cdclk = intel_mode_target_cdclk(dev_priv, state);
+	if (cdclk < 0)
+		return cdclk;
 
 	if (cdclk == dev_priv->cdclk_freq)
 		return 0;
@@ -5981,16 +5997,14 @@ static void valleyview_modeset_global_resources(struct drm_atomic_state *old_sta
 {
 	struct drm_device *dev = old_state->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int max_pixclk = intel_mode_max_pixclk(dev, NULL);
 	int req_cdclk;
 
-	/* The path in intel_mode_max_pixclk() with a NULL atomic state should
+	/* The path in intel_mode_target_cdclk() with a NULL atomic state should
 	 * never fail. */
-	if (WARN_ON(max_pixclk < 0))
+	req_cdclk = intel_mode_target_cdclk(dev_priv, NULL);
+	if (WARN_ON(req_cdclk < 0))
 		return;
 
-	req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
-
 	if (req_cdclk != dev_priv->cdclk_freq) {
 		/*
 		 * FIXME: We can end up here with all power domains off, yet
@@ -9556,15 +9570,13 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
 {
 	struct drm_device *dev = old_state->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int max_pixclk = intel_mode_max_pixclk(dev, NULL);
 	int req_cdclk;
 
 	/* see the comment in valleyview_modeset_global_resources */
-	if (WARN_ON(max_pixclk < 0))
+	req_cdclk = intel_mode_target_cdclk(dev_priv, NULL);
+	if (WARN_ON(req_cdclk < 0))
 		return;
 
-	req_cdclk = broxton_calc_cdclk(dev_priv, max_pixclk);
-
 	if (req_cdclk != dev_priv->cdclk_freq)
 		broxton_set_cdclk(dev, req_cdclk);
 }
-- 
2.1.4

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

[PATCH 2/2] drm/i915/bxt: fix max scaling factor calculation

[Imre Deak]

Atm when calculating the maximum plane scale factor, we use the active
CDCLK rate. The target CDCLK rate for the upcoming modeset may be
different from this, so use the target rate instead.

This fixes the modeset on BXT, where the initial rate was smaller than
the target rate. On SKL we use a fixed CDCLK rate for now, so there this
wasn't an issue.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9b68a5c..15b9208 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5908,7 +5908,10 @@ static int intel_mode_target_cdclk(struct drm_i915_private *dev_priv,
 	if (max_pixclk < 0)
 		return max_pixclk;
 
-	if (IS_BROXTON(dev_priv)) {
+	if (IS_SKYLAKE(dev_priv)) {
+		/* FIXME: calculate the actual target clock rate */
+		cdclk = dev_priv->display.get_display_clock_speed(dev_priv->dev);
+	} else if (IS_BROXTON(dev_priv)) {
 		cdclk = broxton_calc_cdclk(dev_priv, max_pixclk);
 	} else if (IS_VALLEYVIEW(dev_priv)) {
 		cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
@@ -13597,7 +13600,7 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
 	dev = intel_crtc->base.dev;
 	dev_priv = dev->dev_private;
 	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
-	cdclk = dev_priv->display.get_display_clock_speed(dev);
+	cdclk = intel_mode_target_cdclk(dev_priv, crtc_state->base.state);
 
 	if (!crtc_clock || !cdclk)
 		return DRM_PLANE_HELPER_NO_SCALING;
-- 
2.1.4

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

Re: [PATCH 1/2] drm/i915: factor out intel_mode_target_cdclk

[Damien Lespiau]

On Wed, Jun 17, 2015 at 02:00:54PM +0300, Imre Deak wrote:
> For GEN9 the target cdclk frequency is needed during the modeset state check
> phase too, so factor out this functionality.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

I believe the "proper" way to do it is to put what you called the target
CDCLK in the atomic state and use that here. Maarten has a patch to do
that towards the end of his atomic series.

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++++++--------------
>  1 file changed, 30 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2641053..9b68a5c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5899,21 +5899,37 @@ static int intel_mode_max_pixclk(struct drm_device *dev,
>  	return max_pixclk;
>  }
>  
> -static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
> +static int intel_mode_target_cdclk(struct drm_i915_private *dev_priv,
> +				   struct drm_atomic_state *state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	struct drm_crtc *crtc;
> -	struct drm_crtc_state *crtc_state;
> -	int max_pixclk = intel_mode_max_pixclk(state->dev, state);
> -	int cdclk, ret = 0;
> +	int max_pixclk = intel_mode_max_pixclk(dev_priv->dev, state);
> +	int cdclk;
>  
>  	if (max_pixclk < 0)
>  		return max_pixclk;
>  
> -	if (IS_VALLEYVIEW(dev_priv))
> -		cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
> -	else
> +	if (IS_BROXTON(dev_priv)) {
>  		cdclk = broxton_calc_cdclk(dev_priv, max_pixclk);
> +	} else if (IS_VALLEYVIEW(dev_priv)) {
> +		cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
> +	} else {
> +		MISSING_CASE(INTEL_INFO(dev_priv));
> +		cdclk = 0;
> +	}
> +
> +	return cdclk;
> +}
> +
> +static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
> +	int cdclk, ret = 0;
> +
> +	cdclk = intel_mode_target_cdclk(dev_priv, state);
> +	if (cdclk < 0)
> +		return cdclk;
>  
>  	if (cdclk == dev_priv->cdclk_freq)
>  		return 0;
> @@ -5981,16 +5997,14 @@ static void valleyview_modeset_global_resources(struct drm_atomic_state *old_sta
>  {
>  	struct drm_device *dev = old_state->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int max_pixclk = intel_mode_max_pixclk(dev, NULL);
>  	int req_cdclk;
>  
> -	/* The path in intel_mode_max_pixclk() with a NULL atomic state should
> +	/* The path in intel_mode_target_cdclk() with a NULL atomic state should
>  	 * never fail. */
> -	if (WARN_ON(max_pixclk < 0))
> +	req_cdclk = intel_mode_target_cdclk(dev_priv, NULL);
> +	if (WARN_ON(req_cdclk < 0))
>  		return;
>  
> -	req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
> -
>  	if (req_cdclk != dev_priv->cdclk_freq) {
>  		/*
>  		 * FIXME: We can end up here with all power domains off, yet
> @@ -9556,15 +9570,13 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
>  {
>  	struct drm_device *dev = old_state->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int max_pixclk = intel_mode_max_pixclk(dev, NULL);
>  	int req_cdclk;
>  
>  	/* see the comment in valleyview_modeset_global_resources */
> -	if (WARN_ON(max_pixclk < 0))
> +	req_cdclk = intel_mode_target_cdclk(dev_priv, NULL);
> +	if (WARN_ON(req_cdclk < 0))
>  		return;
>  
> -	req_cdclk = broxton_calc_cdclk(dev_priv, max_pixclk);
> -
>  	if (req_cdclk != dev_priv->cdclk_freq)
>  		broxton_set_cdclk(dev, req_cdclk);
>  }
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: factor out intel_mode_target_cdclk

[Imre Deak]

On ma, 2015-06-22 at 14:37 +0100, Damien Lespiau wrote:
> On Wed, Jun 17, 2015 at 02:00:54PM +0300, Imre Deak wrote:
> > For GEN9 the target cdclk frequency is needed during the modeset state check
> > phase too, so factor out this functionality.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> I believe the "proper" way to do it is to put what you called the target
> CDCLK in the atomic state and use that here. Maarten has a patch to do
> that towards the end of his atomic series.

Yes, I was also thinking of using a pre-calculated value, but that
looked like a bigger work. I haven't seen Maarten's patch but that looks
like the way to go (Daniel also pointed to this on IRC). So I can rebase
this once Maarten's patchset lands.

--Imre

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