All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
@ 2019-08-20 11:06 Mika Kahola
  2019-08-20 13:03 ` Ville Syrjälä
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Mika Kahola @ 2019-08-20 11:06 UTC (permalink / raw)
  To: intel-gfx

In order to achieve improved power savings we can tune down CD clock frequency
for sub 4k resolutions. The maximum CD clock frequency for sub 4k
resolutions is set to 172.8 MHz.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 26 +++++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_cdclk.h |  3 +++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index d0bc42e5039c..1d6c7bc79470 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2610,6 +2610,24 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
 		return max_cdclk_freq*90/100;
 }
 
+bool mode_is_4k(struct drm_i915_private *dev_priv)
+{
+	struct intel_crtc *crtc;
+	struct intel_crtc_state *pipe_config;
+
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		pipe_config = to_intel_crtc_state(crtc->base.state);
+
+		if (pipe_config->base.active) {
+			if (pipe_config->pipe_src_w >= WIDTH_4K &&
+			    pipe_config->pipe_src_h >= HEIGHT_4K)
+				return true;
+		}
+	}
+
+	return false;
+}
+
 /**
  * intel_update_max_cdclk - Determine the maximum support CDCLK frequency
  * @dev_priv: i915 device
@@ -2620,7 +2638,13 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
  */
 void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
 {
-	if (IS_ELKHARTLAKE(dev_priv)) {
+	/*
+	 * Use lower CDCLK frequency on Tigerlake when selected
+	 * mode is less than 4k.
+	 */
+	if (INTEL_GEN(dev_priv) >= 12 && !mode_is_4k(dev_priv)) {
+		dev_priv->max_cdclk_freq = 172800;
+	} else if (IS_ELKHARTLAKE(dev_priv)) {
 		if (dev_priv->cdclk.hw.ref == 24000)
 			dev_priv->max_cdclk_freq = 552000;
 		else
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 4d6f7f5f8930..cefb5146ddca 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -15,6 +15,9 @@ struct intel_atomic_state;
 struct intel_cdclk_state;
 struct intel_crtc_state;
 
+#define WIDTH_4K  3860
+#define HEIGHT_4K 2160
+
 int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
 void intel_cdclk_init(struct drm_i915_private *i915);
 void intel_cdclk_uninit(struct drm_i915_private *i915);
-- 
2.17.1

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

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

* Re: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 11:06 [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions Mika Kahola
@ 2019-08-20 13:03 ` Ville Syrjälä
  2019-08-20 13:22   ` Kahola, Mika
  2019-08-20 13:35 ` Shankar, Uma
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2019-08-20 13:03 UTC (permalink / raw)
  To: Mika Kahola; +Cc: intel-gfx

On Tue, Aug 20, 2019 at 02:06:31PM +0300, Mika Kahola wrote:
> In order to achieve improved power savings we can tune down CD clock frequency
> for sub 4k resolutions. The maximum CD clock frequency for sub 4k
> resolutions is set to 172.8 MHz.
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 26 +++++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_cdclk.h |  3 +++
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index d0bc42e5039c..1d6c7bc79470 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2610,6 +2610,24 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>  		return max_cdclk_freq*90/100;
>  }
>  
> +bool mode_is_4k(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_crtc *crtc;
> +	struct intel_crtc_state *pipe_config;
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		pipe_config = to_intel_crtc_state(crtc->base.state);
> +
> +		if (pipe_config->base.active) {
> +			if (pipe_config->pipe_src_w >= WIDTH_4K &&
> +			    pipe_config->pipe_src_h >= HEIGHT_4K)
> +				return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  /**
>   * intel_update_max_cdclk - Determine the maximum support CDCLK frequency
>   * @dev_priv: i915 device
> @@ -2620,7 +2638,13 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>   */
>  void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_ELKHARTLAKE(dev_priv)) {
> +	/*
> +	 * Use lower CDCLK frequency on Tigerlake when selected
> +	 * mode is less than 4k.
> +	 */
> +	if (INTEL_GEN(dev_priv) >= 12 && !mode_is_4k(dev_priv)) {
> +		dev_priv->max_cdclk_freq = 172800;

The maximum is just that, the maximum. It doesn't affect the actual
cdclk chosen (outside of rejecting everything exceeding the max).
And the maximum won't ever change, so trying to calculate it based
on some ephemeral crtc states doesn't make sense.

Given that our policy is to always go for the minimum acceptable cdclk
frequency I don't think there is any work to be done to get proper
power savings for <4k. What is the actual problem you're seeing?

> +	} else if (IS_ELKHARTLAKE(dev_priv)) {
>  		if (dev_priv->cdclk.hw.ref == 24000)
>  			dev_priv->max_cdclk_freq = 552000;
>  		else
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index 4d6f7f5f8930..cefb5146ddca 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -15,6 +15,9 @@ struct intel_atomic_state;
>  struct intel_cdclk_state;
>  struct intel_crtc_state;
>  
> +#define WIDTH_4K  3860
> +#define HEIGHT_4K 2160
> +
>  int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
>  void intel_cdclk_init(struct drm_i915_private *i915);
>  void intel_cdclk_uninit(struct drm_i915_private *i915);
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 13:03 ` Ville Syrjälä
@ 2019-08-20 13:22   ` Kahola, Mika
  2019-08-20 13:44     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Kahola, Mika @ 2019-08-20 13:22 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Tue, 2019-08-20 at 16:03 +0300, Ville Syrjälä wrote:
> On Tue, Aug 20, 2019 at 02:06:31PM +0300, Mika Kahola wrote:
> > In order to achieve improved power savings we can tune down CD
> > clock frequency
> > for sub 4k resolutions. The maximum CD clock frequency for sub 4k
> > resolutions is set to 172.8 MHz.
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 26
> > +++++++++++++++++++++-
> >  drivers/gpu/drm/i915/display/intel_cdclk.h |  3 +++
> >  2 files changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index d0bc42e5039c..1d6c7bc79470 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -2610,6 +2610,24 @@ static int intel_compute_max_dotclk(struct
> > drm_i915_private *dev_priv)
> >  		return max_cdclk_freq*90/100;
> >  }
> >  
> > +bool mode_is_4k(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_crtc *crtc;
> > +	struct intel_crtc_state *pipe_config;
> > +
> > +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> > +		pipe_config = to_intel_crtc_state(crtc->base.state);
> > +
> > +		if (pipe_config->base.active) {
> > +			if (pipe_config->pipe_src_w >= WIDTH_4K &&
> > +			    pipe_config->pipe_src_h >= HEIGHT_4K)
> > +				return true;
> > +		}
> > +	}
> > +
> > +	return false;
> > +}
> > +
> >  /**
> >   * intel_update_max_cdclk - Determine the maximum support CDCLK
> > frequency
> >   * @dev_priv: i915 device
> > @@ -2620,7 +2638,13 @@ static int intel_compute_max_dotclk(struct
> > drm_i915_private *dev_priv)
> >   */
> >  void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
> >  {
> > -	if (IS_ELKHARTLAKE(dev_priv)) {
> > +	/*
> > +	 * Use lower CDCLK frequency on Tigerlake when selected
> > +	 * mode is less than 4k.
> > +	 */
> > +	if (INTEL_GEN(dev_priv) >= 12 && !mode_is_4k(dev_priv)) {
> > +		dev_priv->max_cdclk_freq = 172800;
> 
> The maximum is just that, the maximum. It doesn't affect the actual
> cdclk chosen (outside of rejecting everything exceeding the max).
> And the maximum won't ever change, so trying to calculate it based
> on some ephemeral crtc states doesn't make sense.
> 
> Given that our policy is to always go for the minimum acceptable
> cdclk
> frequency I don't think there is any work to be done to get proper
> power savings for <4k. What is the actual problem you're seeing?
The actual problem is that this is a requested feature for TGL. I
admit, with these suggested optimizations the gains will be marginal.

My interpretation of this feature was that we should not exceed
172.8MHz with the sub 4k modes, hence I'm suggesting in this patch to limit the max cdclock to this number. 

So, how do we get forward? Should I propose that we drop this feature
or should we implement this differently?

> 
> > +	} else if (IS_ELKHARTLAKE(dev_priv)) {
> >  		if (dev_priv->cdclk.hw.ref == 24000)
> >  			dev_priv->max_cdclk_freq = 552000;
> >  		else
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > index 4d6f7f5f8930..cefb5146ddca 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > @@ -15,6 +15,9 @@ struct intel_atomic_state;
> >  struct intel_cdclk_state;
> >  struct intel_crtc_state;
> >  
> > +#define WIDTH_4K  3860
> > +#define HEIGHT_4K 2160
> > +
> >  int intel_crtc_compute_min_cdclk(const struct intel_crtc_state
> > *crtc_state);
> >  void intel_cdclk_init(struct drm_i915_private *i915);
> >  void intel_cdclk_uninit(struct drm_i915_private *i915);
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 11:06 [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions Mika Kahola
  2019-08-20 13:03 ` Ville Syrjälä
@ 2019-08-20 13:35 ` Shankar, Uma
  2019-08-20 13:43   ` Ville Syrjälä
  2019-08-21  6:18   ` Kahola, Mika
  2019-08-20 16:28 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 12+ messages in thread
From: Shankar, Uma @ 2019-08-20 13:35 UTC (permalink / raw)
  To: Kahola, Mika, intel-gfx



>-----Original Message-----
>From: Kahola, Mika
>Sent: Tuesday, August 20, 2019 4:37 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Shankar, Uma <uma.shankar@intel.com>; Kahola, Mika
><mika.kahola@intel.com>
>Subject: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
>
>In order to achieve improved power savings we can tune down CD clock frequency for
>sub 4k resolutions. The maximum CD clock frequency for sub 4k resolutions is set to
>172.8 MHz.
>
>Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 26 +++++++++++++++++++++-
>drivers/gpu/drm/i915/display/intel_cdclk.h |  3 +++
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
>b/drivers/gpu/drm/i915/display/intel_cdclk.c
>index d0bc42e5039c..1d6c7bc79470 100644
>--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>@@ -2610,6 +2610,24 @@ static int intel_compute_max_dotclk(struct
>drm_i915_private *dev_priv)
> 		return max_cdclk_freq*90/100;
> }
>
>+bool mode_is_4k(struct drm_i915_private *dev_priv) {
>+	struct intel_crtc *crtc;
>+	struct intel_crtc_state *pipe_config;
>+
>+	for_each_intel_crtc(&dev_priv->drm, crtc) {
>+		pipe_config = to_intel_crtc_state(crtc->base.state);
>+
>+		if (pipe_config->base.active) {
>+			if (pipe_config->pipe_src_w >= WIDTH_4K &&
>+			    pipe_config->pipe_src_h >= HEIGHT_4K)
>+				return true;
>+		}
>+	}
>+
>+	return false;
>+}
>+
> /**
>  * intel_update_max_cdclk - Determine the maximum support CDCLK frequency
>  * @dev_priv: i915 device
>@@ -2620,7 +2638,13 @@ static int intel_compute_max_dotclk(struct
>drm_i915_private *dev_priv)
>  */
> void intel_update_max_cdclk(struct drm_i915_private *dev_priv)  {
>-	if (IS_ELKHARTLAKE(dev_priv)) {
>+	/*
>+	 * Use lower CDCLK frequency on Tigerlake when selected
>+	 * mode is less than 4k.
>+	 */
>+	if (INTEL_GEN(dev_priv) >= 12 && !mode_is_4k(dev_priv)) {
>+		dev_priv->max_cdclk_freq = 172800;
>+	} else if (IS_ELKHARTLAKE(dev_priv)) {

Setting the max cd clock supported itself to a lower value is not a good idea.
This should return what is the max frequency of cd clock the hardware supports.
Driver is not going to program this to max based on this data.

Actual cd clock which will be programmed in hardware should be based on the
maximum pixel clock we are driving. So if we are not driving 4K and say at 1920x1080, pixel
clock will be less and we should be selecting a lower value of cd clock in that case which
gets programmed in CDCLK_CTL. But max cd clock still remains what maximum the platform can
support. You can check intel_compute_min_cdclk which calculates the minimum cd clock required.

So when we just have 1920x1080@60Hz pixel clock of 148500, 
min cd clock required should be 148500 and nearest higher value of CD Clock will be 172.8Mhz.
Similarly for 4096x2160@60, pixel clock 556188, min cd clock calculated will be 556.188 MHz, thus
getting nearest supported cd clock value of 556 or 648Mhz.

This should be taken care by this logic in driver. 

Also with the current patch, modes like 2560x1600@60, pixel clock 268.5MHz will not work. This will need
cd clock of 324Mhz (or 307, 312 whatever are supported on the platform. 

> 		if (dev_priv->cdclk.hw.ref == 24000)
> 			dev_priv->max_cdclk_freq = 552000;
> 		else
>diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
>b/drivers/gpu/drm/i915/display/intel_cdclk.h
>index 4d6f7f5f8930..cefb5146ddca 100644
>--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
>+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
>@@ -15,6 +15,9 @@ struct intel_atomic_state;  struct intel_cdclk_state;  struct
>intel_crtc_state;
>
>+#define WIDTH_4K  3860
>+#define HEIGHT_4K 2160
>+
> int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);  void
>intel_cdclk_init(struct drm_i915_private *i915);  void intel_cdclk_uninit(struct
>drm_i915_private *i915);
>--
>2.17.1

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

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

* Re: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 13:35 ` Shankar, Uma
@ 2019-08-20 13:43   ` Ville Syrjälä
  2019-08-20 15:06     ` Shankar, Uma
  2019-08-21  6:18   ` Kahola, Mika
  1 sibling, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2019-08-20 13:43 UTC (permalink / raw)
  To: Shankar, Uma; +Cc: intel-gfx

On Tue, Aug 20, 2019 at 01:35:37PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Kahola, Mika
> >Sent: Tuesday, August 20, 2019 4:37 PM
> >To: intel-gfx@lists.freedesktop.org
> >Cc: Shankar, Uma <uma.shankar@intel.com>; Kahola, Mika
> ><mika.kahola@intel.com>
> >Subject: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
> >
> >In order to achieve improved power savings we can tune down CD clock frequency for
> >sub 4k resolutions. The maximum CD clock frequency for sub 4k resolutions is set to
> >172.8 MHz.
> >
> >Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> >---
> > drivers/gpu/drm/i915/display/intel_cdclk.c | 26 +++++++++++++++++++++-
> >drivers/gpu/drm/i915/display/intel_cdclk.h |  3 +++
> > 2 files changed, 28 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> >b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >index d0bc42e5039c..1d6c7bc79470 100644
> >--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> >+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >@@ -2610,6 +2610,24 @@ static int intel_compute_max_dotclk(struct
> >drm_i915_private *dev_priv)
> > 		return max_cdclk_freq*90/100;
> > }
> >
> >+bool mode_is_4k(struct drm_i915_private *dev_priv) {
> >+	struct intel_crtc *crtc;
> >+	struct intel_crtc_state *pipe_config;
> >+
> >+	for_each_intel_crtc(&dev_priv->drm, crtc) {
> >+		pipe_config = to_intel_crtc_state(crtc->base.state);
> >+
> >+		if (pipe_config->base.active) {
> >+			if (pipe_config->pipe_src_w >= WIDTH_4K &&
> >+			    pipe_config->pipe_src_h >= HEIGHT_4K)
> >+				return true;
> >+		}
> >+	}
> >+
> >+	return false;
> >+}
> >+
> > /**
> >  * intel_update_max_cdclk - Determine the maximum support CDCLK frequency
> >  * @dev_priv: i915 device
> >@@ -2620,7 +2638,13 @@ static int intel_compute_max_dotclk(struct
> >drm_i915_private *dev_priv)
> >  */
> > void intel_update_max_cdclk(struct drm_i915_private *dev_priv)  {
> >-	if (IS_ELKHARTLAKE(dev_priv)) {
> >+	/*
> >+	 * Use lower CDCLK frequency on Tigerlake when selected
> >+	 * mode is less than 4k.
> >+	 */
> >+	if (INTEL_GEN(dev_priv) >= 12 && !mode_is_4k(dev_priv)) {
> >+		dev_priv->max_cdclk_freq = 172800;
> >+	} else if (IS_ELKHARTLAKE(dev_priv)) {
> 
> Setting the max cd clock supported itself to a lower value is not a good idea.
> This should return what is the max frequency of cd clock the hardware supports.
> Driver is not going to program this to max based on this data.
> 
> Actual cd clock which will be programmed in hardware should be based on the
> maximum pixel clock we are driving. So if we are not driving 4K and say at 1920x1080, pixel
> clock will be less and we should be selecting a lower value of cd clock in that case which
> gets programmed in CDCLK_CTL. But max cd clock still remains what maximum the platform can
> support. You can check intel_compute_min_cdclk which calculates the minimum cd clock required.
> 
> So when we just have 1920x1080@60Hz pixel clock of 148500, 
> min cd clock required should be 148500 and nearest higher value of CD Clock will be 172.8Mhz.
> Similarly for 4096x2160@60, pixel clock 556188, min cd clock calculated will be 556.188 MHz, thus
> getting nearest supported cd clock value of 556 or 648Mhz.
> 
> This should be taken care by this logic in driver. 
> 
> Also with the current patch, modes like 2560x1600@60, pixel clock 268.5MHz will not work. This will need
> cd clock of 324Mhz (or 307, 312 whatever are supported on the platform. 

glk+ pump out two pixels per clock, so we need half of what you said.

> 
> > 		if (dev_priv->cdclk.hw.ref == 24000)
> > 			dev_priv->max_cdclk_freq = 552000;
> > 		else
> >diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
> >b/drivers/gpu/drm/i915/display/intel_cdclk.h
> >index 4d6f7f5f8930..cefb5146ddca 100644
> >--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> >+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> >@@ -15,6 +15,9 @@ struct intel_atomic_state;  struct intel_cdclk_state;  struct
> >intel_crtc_state;
> >
> >+#define WIDTH_4K  3860
> >+#define HEIGHT_4K 2160
> >+
> > int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);  void
> >intel_cdclk_init(struct drm_i915_private *i915);  void intel_cdclk_uninit(struct
> >drm_i915_private *i915);
> >--
> >2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 13:22   ` Kahola, Mika
@ 2019-08-20 13:44     ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-08-20 13:44 UTC (permalink / raw)
  To: Kahola, Mika; +Cc: intel-gfx

On Tue, Aug 20, 2019 at 01:22:00PM +0000, Kahola, Mika wrote:
> On Tue, 2019-08-20 at 16:03 +0300, Ville Syrjälä wrote:
> > On Tue, Aug 20, 2019 at 02:06:31PM +0300, Mika Kahola wrote:
> > > In order to achieve improved power savings we can tune down CD
> > > clock frequency
> > > for sub 4k resolutions. The maximum CD clock frequency for sub 4k
> > > resolutions is set to 172.8 MHz.
> > > 
> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cdclk.c | 26
> > > +++++++++++++++++++++-
> > >  drivers/gpu/drm/i915/display/intel_cdclk.h |  3 +++
> > >  2 files changed, 28 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > index d0bc42e5039c..1d6c7bc79470 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > @@ -2610,6 +2610,24 @@ static int intel_compute_max_dotclk(struct
> > > drm_i915_private *dev_priv)
> > >  		return max_cdclk_freq*90/100;
> > >  }
> > >  
> > > +bool mode_is_4k(struct drm_i915_private *dev_priv)
> > > +{
> > > +	struct intel_crtc *crtc;
> > > +	struct intel_crtc_state *pipe_config;
> > > +
> > > +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> > > +		pipe_config = to_intel_crtc_state(crtc->base.state);
> > > +
> > > +		if (pipe_config->base.active) {
> > > +			if (pipe_config->pipe_src_w >= WIDTH_4K &&
> > > +			    pipe_config->pipe_src_h >= HEIGHT_4K)
> > > +				return true;
> > > +		}
> > > +	}
> > > +
> > > +	return false;
> > > +}
> > > +
> > >  /**
> > >   * intel_update_max_cdclk - Determine the maximum support CDCLK
> > > frequency
> > >   * @dev_priv: i915 device
> > > @@ -2620,7 +2638,13 @@ static int intel_compute_max_dotclk(struct
> > > drm_i915_private *dev_priv)
> > >   */
> > >  void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
> > >  {
> > > -	if (IS_ELKHARTLAKE(dev_priv)) {
> > > +	/*
> > > +	 * Use lower CDCLK frequency on Tigerlake when selected
> > > +	 * mode is less than 4k.
> > > +	 */
> > > +	if (INTEL_GEN(dev_priv) >= 12 && !mode_is_4k(dev_priv)) {
> > > +		dev_priv->max_cdclk_freq = 172800;
> > 
> > The maximum is just that, the maximum. It doesn't affect the actual
> > cdclk chosen (outside of rejecting everything exceeding the max).
> > And the maximum won't ever change, so trying to calculate it based
> > on some ephemeral crtc states doesn't make sense.
> > 
> > Given that our policy is to always go for the minimum acceptable
> > cdclk
> > frequency I don't think there is any work to be done to get proper
> > power savings for <4k. What is the actual problem you're seeing?
> The actual problem is that this is a requested feature for TGL. I
> admit, with these suggested optimizations the gains will be marginal.
> 
> My interpretation of this feature was that we should not exceed
> 172.8MHz with the sub 4k modes, hence I'm suggesting in this patch to limit the max cdclock to this number. 
> 
> So, how do we get forward? Should I propose that we drop this feature
> or should we implement this differently?

There is nothing to implement. The current policy already picks the
minimum cdclk that will work.

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

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

* Re: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 13:43   ` Ville Syrjälä
@ 2019-08-20 15:06     ` Shankar, Uma
  0 siblings, 0 replies; 12+ messages in thread
From: Shankar, Uma @ 2019-08-20 15:06 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


>> >-----Original Message-----
>> >From: Kahola, Mika
>> >Sent: Tuesday, August 20, 2019 4:37 PM
>> >To: intel-gfx@lists.freedesktop.org
>> >Cc: Shankar, Uma <uma.shankar@intel.com>; Kahola, Mika
>> ><mika.kahola@intel.com>
>> >Subject: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
>> >
>> >In order to achieve improved power savings we can tune down CD clock
>> >frequency for sub 4k resolutions. The maximum CD clock frequency for
>> >sub 4k resolutions is set to
>> >172.8 MHz.
>> >
>> >Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> >---
>> > drivers/gpu/drm/i915/display/intel_cdclk.c | 26
>> >+++++++++++++++++++++- drivers/gpu/drm/i915/display/intel_cdclk.h |
>> >3 +++
>> > 2 files changed, 28 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> >b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> >index d0bc42e5039c..1d6c7bc79470 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> >+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> >@@ -2610,6 +2610,24 @@ static int intel_compute_max_dotclk(struct
>> >drm_i915_private *dev_priv)
>> > 		return max_cdclk_freq*90/100;
>> > }
>> >
>> >+bool mode_is_4k(struct drm_i915_private *dev_priv) {
>> >+	struct intel_crtc *crtc;
>> >+	struct intel_crtc_state *pipe_config;
>> >+
>> >+	for_each_intel_crtc(&dev_priv->drm, crtc) {
>> >+		pipe_config = to_intel_crtc_state(crtc->base.state);
>> >+
>> >+		if (pipe_config->base.active) {
>> >+			if (pipe_config->pipe_src_w >= WIDTH_4K &&
>> >+			    pipe_config->pipe_src_h >= HEIGHT_4K)
>> >+				return true;
>> >+		}
>> >+	}
>> >+
>> >+	return false;
>> >+}
>> >+
>> > /**
>> >  * intel_update_max_cdclk - Determine the maximum support CDCLK
>> >frequency
>> >  * @dev_priv: i915 device
>> >@@ -2620,7 +2638,13 @@ static int intel_compute_max_dotclk(struct
>> >drm_i915_private *dev_priv)
>> >  */
>> > void intel_update_max_cdclk(struct drm_i915_private *dev_priv)  {
>> >-	if (IS_ELKHARTLAKE(dev_priv)) {
>> >+	/*
>> >+	 * Use lower CDCLK frequency on Tigerlake when selected
>> >+	 * mode is less than 4k.
>> >+	 */
>> >+	if (INTEL_GEN(dev_priv) >= 12 && !mode_is_4k(dev_priv)) {
>> >+		dev_priv->max_cdclk_freq = 172800;
>> >+	} else if (IS_ELKHARTLAKE(dev_priv)) {
>>
>> Setting the max cd clock supported itself to a lower value is not a good idea.
>> This should return what is the max frequency of cd clock the hardware supports.
>> Driver is not going to program this to max based on this data.
>>
>> Actual cd clock which will be programmed in hardware should be based
>> on the maximum pixel clock we are driving. So if we are not driving 4K
>> and say at 1920x1080, pixel clock will be less and we should be
>> selecting a lower value of cd clock in that case which gets programmed
>> in CDCLK_CTL. But max cd clock still remains what maximum the platform can
>support. You can check intel_compute_min_cdclk which calculates the minimum cd
>clock required.
>>
>> So when we just have 1920x1080@60Hz pixel clock of 148500, min cd
>> clock required should be 148500 and nearest higher value of CD Clock will be
>172.8Mhz.
>> Similarly for 4096x2160@60, pixel clock 556188, min cd clock
>> calculated will be 556.188 MHz, thus getting nearest supported cd clock value of
>556 or 648Mhz.
>>
>> This should be taken care by this logic in driver.
>>
>> Also with the current patch, modes like 2560x1600@60, pixel clock
>> 268.5MHz will not work. This will need cd clock of 324Mhz (or 307, 312 whatever
>are supported on the platform.
>
>glk+ pump out two pixels per clock, so we need half of what you said.

Yeah correct Ville. From GLK+ its 2 pixels per clock.

>>
>> > 		if (dev_priv->cdclk.hw.ref == 24000)
>> > 			dev_priv->max_cdclk_freq = 552000;
>> > 		else
>> >diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> >b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> >index 4d6f7f5f8930..cefb5146ddca 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> >+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> >@@ -15,6 +15,9 @@ struct intel_atomic_state;  struct
>> >intel_cdclk_state;  struct intel_crtc_state;
>> >
>> >+#define WIDTH_4K  3860
>> >+#define HEIGHT_4K 2160
>> >+
>> > int intel_crtc_compute_min_cdclk(const struct intel_crtc_state
>> >*crtc_state);  void intel_cdclk_init(struct drm_i915_private *i915);
>> >void intel_cdclk_uninit(struct drm_i915_private *i915);
>> >--
>> >2.17.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 11:06 [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions Mika Kahola
  2019-08-20 13:03 ` Ville Syrjälä
  2019-08-20 13:35 ` Shankar, Uma
@ 2019-08-20 16:28 ` Patchwork
  2019-08-20 16:29 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-08-20 16:28 UTC (permalink / raw)
  To: Kahola, Mika; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/tgl: Lower cdclk for sub 4k resolutions
URL   : https://patchwork.freedesktop.org/series/65475/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
0fb206f34d2d drm/i915/tgl: Lower cdclk for sub 4k resolutions
-:6: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#6: 
In order to achieve improved power savings we can tune down CD clock frequency

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

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

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

* ✗ Fi.CI.SPARSE: warning for drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 11:06 [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions Mika Kahola
                   ` (2 preceding siblings ...)
  2019-08-20 16:28 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2019-08-20 16:29 ` Patchwork
  2019-08-20 17:51 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-08-21  5:29 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-08-20 16:29 UTC (permalink / raw)
  To: Kahola, Mika; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/tgl: Lower cdclk for sub 4k resolutions
URL   : https://patchwork.freedesktop.org/series/65475/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915/tgl: Lower cdclk for sub 4k resolutions
+drivers/gpu/drm/i915/display/intel_cdclk.c:2613:6: warning: symbol 'mode_is_4k' was not declared. Should it be static?

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

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

* ✓ Fi.CI.BAT: success for drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 11:06 [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions Mika Kahola
                   ` (3 preceding siblings ...)
  2019-08-20 16:29 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-08-20 17:51 ` Patchwork
  2019-08-21  5:29 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-08-20 17:51 UTC (permalink / raw)
  To: Kahola, Mika; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/tgl: Lower cdclk for sub 4k resolutions
URL   : https://patchwork.freedesktop.org/series/65475/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6748 -> Patchwork_14100
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_switch@rcs0:
    - fi-icl-u2:          [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/fi-icl-u2/igt@gem_ctx_switch@rcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/fi-icl-u2/igt@gem_ctx_switch@rcs0.html

  * igt@gem_sync@basic-store-each:
    - fi-cfl-8109u:       [PASS][3] -> [INCOMPLETE][4] ([fdo#111427])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/fi-cfl-8109u/igt@gem_sync@basic-store-each.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/fi-cfl-8109u/igt@gem_sync@basic-store-each.html

  * igt@i915_selftest@live_execlists:
    - fi-skl-gvtdvm:      [PASS][5] -> [DMESG-FAIL][6] ([fdo#111108])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gtt:
    - fi-bxt-dsi:         [PASS][7] -> [INCOMPLETE][8] ([fdo#103927])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/fi-bxt-dsi/igt@i915_selftest@live_gtt.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/fi-bxt-dsi/igt@i915_selftest@live_gtt.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_coherency:
    - fi-icl-u3:          [INCOMPLETE][9] ([fdo#107713]) -> [PASS][10] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/fi-icl-u3/igt@i915_selftest@live_coherency.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/fi-icl-u3/igt@i915_selftest@live_coherency.html

  * igt@prime_vgem@basic-fence-wait-default:
    - fi-icl-u3:          [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12] +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/fi-icl-u3/igt@prime_vgem@basic-fence-wait-default.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/fi-icl-u3/igt@prime_vgem@basic-fence-wait-default.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#111108]: https://bugs.freedesktop.org/show_bug.cgi?id=111108
  [fdo#111427]: https://bugs.freedesktop.org/show_bug.cgi?id=111427


Participating hosts (54 -> 46)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (9): fi-kbl-soraka fi-icl-u4 fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6748 -> Patchwork_14100

  CI-20190529: 20190529
  CI_DRM_6748: 718169f0c595f45d68bff45ed31517014d4d9419 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14100: 0fb206f34d2d8b961832f8eaad16573f5504db20 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0fb206f34d2d drm/i915/tgl: Lower cdclk for sub 4k resolutions

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 11:06 [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions Mika Kahola
                   ` (4 preceding siblings ...)
  2019-08-20 17:51 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-08-21  5:29 ` Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-08-21  5:29 UTC (permalink / raw)
  To: Kahola, Mika; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/tgl: Lower cdclk for sub 4k resolutions
URL   : https://patchwork.freedesktop.org/series/65475/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6748_full -> Patchwork_14100_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110841])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276]) +16 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb1/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb7/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_exec_schedule@reorder-wide-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#111325]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb3/igt@gem_exec_schedule@reorder-wide-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb1/igt@gem_exec_schedule@reorder-wide-bsd.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +6 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-apl6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-apl8/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#105363])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          [PASS][11] -> [INCOMPLETE][12] ([fdo#109507])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-skl6/igt@kms_flip@flip-vs-suspend.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-skl6/igt@kms_flip@flip-vs-suspend.html
    - shard-iclb:         [PASS][13] -> [INCOMPLETE][14] ([fdo#107713] / [fdo#109507])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb3/igt@kms_flip@flip-vs-suspend.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb3/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-shrfb-scaledprimary:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#103167]) +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-shrfb-scaledprimary.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-shrfb-scaledprimary.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-skl:          [PASS][17] -> [INCOMPLETE][18] ([fdo#104108] / [fdo#106978])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-skl8/igt@kms_frontbuffer_tracking@psr-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-skl3/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#108145] / [fdo#110403])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([fdo#103166])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-x.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [DMESG-WARN][23] ([fdo#108566]) -> [PASS][24] +4 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-apl3/igt@gem_ctx_isolation@rcs0-s3.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-apl5/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_ctx_isolation@vcs0-s3:
    - shard-skl:          [INCOMPLETE][25] ([fdo#104108]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-skl4/igt@gem_ctx_isolation@vcs0-s3.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-skl4/igt@gem_ctx_isolation@vcs0-s3.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [SKIP][27] ([fdo#111325]) -> [PASS][28] +6 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb2/igt@gem_exec_schedule@wide-bsd.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb5/igt@gem_exec_schedule@wide-bsd.html

  * igt@i915_selftest@live_hangcheck:
    - shard-kbl:          [INCOMPLETE][29] ([fdo#103665] / [fdo#108744]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-kbl3/igt@i915_selftest@live_hangcheck.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-kbl7/igt@i915_selftest@live_hangcheck.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [INCOMPLETE][31] ([fdo#103927]) -> [PASS][32] +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-apl8/igt@i915_suspend@sysfs-reader.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-apl4/igt@i915_suspend@sysfs-reader.html
    - shard-iclb:         [INCOMPLETE][33] ([fdo#107713]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb7/igt@i915_suspend@sysfs-reader.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb2/igt@i915_suspend@sysfs-reader.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][35] ([fdo#105363]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-glk5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-glk2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-badstride:
    - shard-iclb:         [FAIL][37] ([fdo#103167]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-badstride.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-badstride.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [SKIP][39] ([fdo#109441]) -> [PASS][40] +1 similar issue
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb8/igt@kms_psr@psr2_basic.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb2/igt@kms_psr@psr2_basic.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][41] ([fdo#99912]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-kbl7/igt@kms_setmode@basic.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-kbl2/igt@kms_setmode@basic.html

  * igt@perf_pmu@rc6:
    - shard-kbl:          [SKIP][43] ([fdo#109271]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-kbl3/igt@perf_pmu@rc6.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-kbl6/igt@perf_pmu@rc6.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [SKIP][45] ([fdo#109276]) -> [PASS][46] +18 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb8/igt@prime_vgem@fence-wait-bsd2.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb2/igt@prime_vgem@fence-wait-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [SKIP][47] ([fdo#109276]) -> [FAIL][48] ([fdo#111329])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb1/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][49] ([fdo#111330]) -> [SKIP][50] ([fdo#109276]) +2 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6748/shard-iclb4/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14100/shard-iclb8/igt@gem_mocs_settings@mocs-reset-bsd2.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [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#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6748 -> Patchwork_14100

  CI-20190529: 20190529
  CI_DRM_6748: 718169f0c595f45d68bff45ed31517014d4d9419 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14100: 0fb206f34d2d8b961832f8eaad16573f5504db20 @ 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_14100/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
  2019-08-20 13:35 ` Shankar, Uma
  2019-08-20 13:43   ` Ville Syrjälä
@ 2019-08-21  6:18   ` Kahola, Mika
  1 sibling, 0 replies; 12+ messages in thread
From: Kahola, Mika @ 2019-08-21  6:18 UTC (permalink / raw)
  To: Shankar, Uma, intel-gfx

On Tue, 2019-08-20 at 19:05 +0530, Shankar, Uma wrote:
> > -----Original Message-----
> > From: Kahola, Mika
> > Sent: Tuesday, August 20, 2019 4:37 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Shankar, Uma <uma.shankar@intel.com>; Kahola, Mika
> > <mika.kahola@intel.com>
> > Subject: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
> > 
> > In order to achieve improved power savings we can tune down CD
> > clock frequency for
> > sub 4k resolutions. The maximum CD clock frequency for sub 4k
> > resolutions is set to
> > 172.8 MHz.
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_cdclk.c | 26
> > +++++++++++++++++++++-
> > drivers/gpu/drm/i915/display/intel_cdclk.h |  3 +++
> > 2 files changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index d0bc42e5039c..1d6c7bc79470 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -2610,6 +2610,24 @@ static int intel_compute_max_dotclk(struct
> > drm_i915_private *dev_priv)
> > 		return max_cdclk_freq*90/100;
> > }
> > 
> > +bool mode_is_4k(struct drm_i915_private *dev_priv) {
> > +	struct intel_crtc *crtc;
> > +	struct intel_crtc_state *pipe_config;
> > +
> > +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> > +		pipe_config = to_intel_crtc_state(crtc->base.state);
> > +
> > +		if (pipe_config->base.active) {
> > +			if (pipe_config->pipe_src_w >= WIDTH_4K &&
> > +			    pipe_config->pipe_src_h >= HEIGHT_4K)
> > +				return true;
> > +		}
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > /**
> >  * intel_update_max_cdclk - Determine the maximum support CDCLK
> > frequency
> >  * @dev_priv: i915 device
> > @@ -2620,7 +2638,13 @@ static int intel_compute_max_dotclk(struct
> > drm_i915_private *dev_priv)
> >  */
> > void intel_update_max_cdclk(struct drm_i915_private *dev_priv)  {
> > -	if (IS_ELKHARTLAKE(dev_priv)) {
> > +	/*
> > +	 * Use lower CDCLK frequency on Tigerlake when selected
> > +	 * mode is less than 4k.
> > +	 */
> > +	if (INTEL_GEN(dev_priv) >= 12 && !mode_is_4k(dev_priv)) {
> > +		dev_priv->max_cdclk_freq = 172800;
> > +	} else if (IS_ELKHARTLAKE(dev_priv)) {
> 
> Setting the max cd clock supported itself to a lower value is not a
> good idea.
> This should return what is the max frequency of cd clock the hardware
> supports.
> Driver is not going to program this to max based on this data.
> 
> Actual cd clock which will be programmed in hardware should be based
> on the
> maximum pixel clock we are driving. So if we are not driving 4K and
> say at 1920x1080, pixel
> clock will be less and we should be selecting a lower value of cd
> clock in that case which
> gets programmed in CDCLK_CTL. But max cd clock still remains what
> maximum the platform can
> support. You can check intel_compute_min_cdclk which calculates the
> minimum cd clock required.
> 
> So when we just have 1920x1080@60Hz pixel clock of 148500, 
> min cd clock required should be 148500 and nearest higher value of CD
> Clock will be 172.8Mhz.
> Similarly for 4096x2160@60, pixel clock 556188, min cd clock
> calculated will be 556.188 MHz, thus
> getting nearest supported cd clock value of 556 or 648Mhz.
> 
> This should be taken care by this logic in driver. 
> 
> Also with the current patch, modes like 2560x1600@60, pixel clock
> 268.5MHz will not work. This will need
> cd clock of 324Mhz (or 307, 312 whatever are supported on the
> platform. 
Right. The dev_priv->max_cdclk_freq was HW max cdclock. We don't want
to mess with that. Since we already pick up the minimum cdclock, the
feature is already built-in cd clock selection process. Thanks for
clarification!

I'll propose that we drop this task since its obsolete.

> 
> > 		if (dev_priv->cdclk.hw.ref == 24000)
> > 			dev_priv->max_cdclk_freq = 552000;
> > 		else
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > index 4d6f7f5f8930..cefb5146ddca 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > @@ -15,6 +15,9 @@ struct intel_atomic_state;  struct
> > intel_cdclk_state;  struct
> > intel_crtc_state;
> > 
> > +#define WIDTH_4K  3860
> > +#define HEIGHT_4K 2160
> > +
> > int intel_crtc_compute_min_cdclk(const struct intel_crtc_state
> > *crtc_state);  void
> > intel_cdclk_init(struct drm_i915_private *i915);  void
> > intel_cdclk_uninit(struct
> > drm_i915_private *i915);
> > --
> > 2.17.1
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2019-08-21  6:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-20 11:06 [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions Mika Kahola
2019-08-20 13:03 ` Ville Syrjälä
2019-08-20 13:22   ` Kahola, Mika
2019-08-20 13:44     ` Ville Syrjälä
2019-08-20 13:35 ` Shankar, Uma
2019-08-20 13:43   ` Ville Syrjälä
2019-08-20 15:06     ` Shankar, Uma
2019-08-21  6:18   ` Kahola, Mika
2019-08-20 16:28 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-08-20 16:29 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-08-20 17:51 ` ✓ Fi.CI.BAT: success " Patchwork
2019-08-21  5:29 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.