Re: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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