public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/skl: CDCLK change during modeset based on VCO in use.
@ 2015-11-19 17:20 clinton.a.taylor
  2015-11-20 13:55 ` Ville Syrjälä
  0 siblings, 1 reply; 6+ messages in thread
From: clinton.a.taylor @ 2015-11-19 17:20 UTC (permalink / raw)
  To: Intel-gfx

From: Clint Taylor <clinton.a.taylor@intel.com>

Add SKL and KBL cdclk changes during modeset. Taking into account new
linkrates available using 8640 VCO.

Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   68 ++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2d7ea95..bed03cb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9727,6 +9727,69 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 	broadwell_set_cdclk(dev, req_cdclk);
 }
 
+static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->dev);
+	int max_pixclk = ilk_max_pixel_rate(state);
+	int cdclk;
+	uint32_t linkrate;
+
+	linkrate = (I915_READ(DPLL_CTRL1) &
+		    DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
+
+	/*
+	* FIXME should also account for plane ratio
+	* once 64bpp pixel formats are supported.
+	*/
+
+	if (linkrate == DPLL_CTRL1_LINK_RATE_2160 ||
+	    linkrate == DPLL_CTRL1_LINK_RATE_1080) {
+		/* vco 8640 */
+		if (max_pixclk > 540000)
+			cdclk = 617140;
+		else if (max_pixclk > 432000)
+			cdclk = 540000;
+		else if (max_pixclk > 308570)
+			cdclk = 432000;
+		else
+			cdclk = 308570;
+	}
+	else {
+		/* VCO 8100 */
+		if (max_pixclk > 540000)
+			cdclk = 675000;
+		else if (max_pixclk > 450000)
+			cdclk = 540000;
+		else if (max_pixclk > 337500)
+			cdclk = 450000;
+		else
+			cdclk = 337500;
+	}
+
+	/*
+	 * FIXME move the cdclk caclulation to
+	 * compute_config() so we can fail gracefully.
+	 */
+	if (cdclk > dev_priv->max_cdclk_freq) {
+		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
+			  cdclk, dev_priv->max_cdclk_freq);
+		cdclk = dev_priv->max_cdclk_freq;
+	}
+
+	to_intel_atomic_state(state)->cdclk = cdclk;
+
+	return 0;
+}
+
+static void skl_modeset_commit_cdclk(struct drm_atomic_state *old_state)
+{
+	struct drm_device *dev = old_state->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned int req_cdclk = to_intel_atomic_state(old_state)->cdclk;
+
+	skl_set_cdclk(dev_priv, req_cdclk);
+}
+
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 				      struct intel_crtc_state *crtc_state)
 {
@@ -14831,6 +14894,11 @@ static void intel_init_display(struct drm_device *dev)
 			broxton_modeset_commit_cdclk;
 		dev_priv->display.modeset_calc_cdclk =
 			broxton_modeset_calc_cdclk;
+	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
+		dev_priv->display.modeset_commit_cdclk =
+			skl_modeset_commit_cdclk;
+		dev_priv->display.modeset_calc_cdclk =
+			skl_modeset_calc_cdclk;
 	}
 
 	switch (INTEL_INFO(dev)->gen) {
-- 
1.7.9.5

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

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

* Re: [PATCH] drm/i915/skl: CDCLK change during modeset based on VCO in use.
  2015-11-19 17:20 [PATCH] drm/i915/skl: CDCLK change during modeset based on VCO in use clinton.a.taylor
@ 2015-11-20 13:55 ` Ville Syrjälä
  2015-11-20 18:10   ` Clint Taylor
  2015-11-20 19:03   ` Daniel Stone
  0 siblings, 2 replies; 6+ messages in thread
From: Ville Syrjälä @ 2015-11-20 13:55 UTC (permalink / raw)
  To: clinton.a.taylor; +Cc: Shobhit Kumar, Intel-gfx

On Thu, Nov 19, 2015 at 09:20:16AM -0800, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
> 
> Add SKL and KBL cdclk changes during modeset. Taking into account new
> linkrates available using 8640 VCO.
> 
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   68 ++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2d7ea95..bed03cb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9727,6 +9727,69 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>  	broadwell_set_cdclk(dev, req_cdclk);
>  }
>  
> +static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> +	int max_pixclk = ilk_max_pixel_rate(state);
> +	int cdclk;
> +	uint32_t linkrate;
> +
> +	linkrate = (I915_READ(DPLL_CTRL1) &
> +		    DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;

I don't think we should read this from the hardware here. Instead
we should stash the proper vco somewhere under dev_priv. Well, we
already have the boot_cdclk there, which is more or less just that.
What's really missing is code to fix up our initial boot_cdclk
choice if it turns out to be wrong. Where would it get fixed? I
assume we'd do that during/after eDP probing since then we should
know what link rate we want to use.

> +
> +	/*
> +	* FIXME should also account for plane ratio
> +	* once 64bpp pixel formats are supported.
> +	*/
> +
> +	if (linkrate == DPLL_CTRL1_LINK_RATE_2160 ||
> +	    linkrate == DPLL_CTRL1_LINK_RATE_1080) {
> +		/* vco 8640 */
> +		if (max_pixclk > 540000)
> +			cdclk = 617140;
> +		else if (max_pixclk > 432000)
> +			cdclk = 540000;
> +		else if (max_pixclk > 308570)
> +			cdclk = 432000;
> +		else
> +			cdclk = 308570;
> +	}
> +	else {
> +		/* VCO 8100 */
> +		if (max_pixclk > 540000)
> +			cdclk = 675000;
> +		else if (max_pixclk > 450000)
> +			cdclk = 540000;
> +		else if (max_pixclk > 337500)
> +			cdclk = 450000;
> +		else
> +			cdclk = 337500;
> +	}
> +
> +	/*
> +	 * FIXME move the cdclk caclulation to
> +	 * compute_config() so we can fail gracefully.
> +	 */
> +	if (cdclk > dev_priv->max_cdclk_freq) {
> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> +			  cdclk, dev_priv->max_cdclk_freq);
> +		cdclk = dev_priv->max_cdclk_freq;
> +	}
> +
> +	to_intel_atomic_state(state)->cdclk = cdclk;
> +
> +	return 0;
> +}
> +
> +static void skl_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> +{
> +	struct drm_device *dev = old_state->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	unsigned int req_cdclk = to_intel_atomic_state(old_state)->cdclk;
> +
> +	skl_set_cdclk(dev_priv, req_cdclk);
> +}
> +
>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>  				      struct intel_crtc_state *crtc_state)
>  {
> @@ -14831,6 +14894,11 @@ static void intel_init_display(struct drm_device *dev)
>  			broxton_modeset_commit_cdclk;
>  		dev_priv->display.modeset_calc_cdclk =
>  			broxton_modeset_calc_cdclk;
> +	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
> +		dev_priv->display.modeset_commit_cdclk =
> +			skl_modeset_commit_cdclk;
> +		dev_priv->display.modeset_calc_cdclk =
> +			skl_modeset_calc_cdclk;
>  	}
>  
>  	switch (INTEL_INFO(dev)->gen) {
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH] drm/i915/skl: CDCLK change during modeset based on VCO in use.
  2015-11-20 13:55 ` Ville Syrjälä
@ 2015-11-20 18:10   ` Clint Taylor
  2015-11-20 18:47     ` Ville Syrjälä
  2015-11-20 19:03   ` Daniel Stone
  1 sibling, 1 reply; 6+ messages in thread
From: Clint Taylor @ 2015-11-20 18:10 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Shobhit Kumar, Intel-gfx

On 11/20/2015 05:55 AM, Ville Syrjälä wrote:
> On Thu, Nov 19, 2015 at 09:20:16AM -0800, clinton.a.taylor@intel.com wrote:
>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> Add SKL and KBL cdclk changes during modeset. Taking into account new
>> linkrates available using 8640 VCO.
>>
>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |   68 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 68 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 2d7ea95..bed03cb 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9727,6 +9727,69 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>>   	broadwell_set_cdclk(dev, req_cdclk);
>>   }
>>
>> +static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
>> +	int max_pixclk = ilk_max_pixel_rate(state);
>> +	int cdclk;
>> +	uint32_t linkrate;
>> +
>> +	linkrate = (I915_READ(DPLL_CTRL1) &
>> +		    DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
>
> I don't think we should read this from the hardware here. Instead
> we should stash the proper vco somewhere under dev_priv. Well, we

I have no problem stuffing the proper VCO based on the link rate 
determined in skl_edp_set_pll_config(). I think just storing a uint32 of 
the required rate would be enough as we know 216000 and 432000 require 
the 8640 VCO.

static const int skl_rates[] = { 162000, 216000, 270000,
				  324000, 432000, 540000 };


> already have the boot_cdclk there, which is more or less just that.
> What's really missing is code to fix up our initial boot_cdclk
> choice if it turns out to be wrong. Where would it get fixed? I
> assume we'd do that during/after eDP probing since then we should
> know what link rate we want to use.

boot_cdclk is irrelevant as soon as boot has completed. I agree we need 
to fixup cdclk in cases when incorrect like at boot or during resume. 
Both cases are outside the scope of this patch which is just to fix 
cdclk during modeset when the boot_cdclk is < required CDCLK after an 
HPD event.

>
>> +
>> +	/*
>> +	* FIXME should also account for plane ratio
>> +	* once 64bpp pixel formats are supported.
>> +	*/
>> +
>> +	if (linkrate == DPLL_CTRL1_LINK_RATE_2160 ||
>> +	    linkrate == DPLL_CTRL1_LINK_RATE_1080) {
>> +		/* vco 8640 */
>> +		if (max_pixclk > 540000)
>> +			cdclk = 617140;
>> +		else if (max_pixclk > 432000)
>> +			cdclk = 540000;
>> +		else if (max_pixclk > 308570)
>> +			cdclk = 432000;
>> +		else
>> +			cdclk = 308570;
>> +	}
>> +	else {
>> +		/* VCO 8100 */
>> +		if (max_pixclk > 540000)
>> +			cdclk = 675000;
>> +		else if (max_pixclk > 450000)
>> +			cdclk = 540000;
>> +		else if (max_pixclk > 337500)
>> +			cdclk = 450000;
>> +		else
>> +			cdclk = 337500;
>> +	}
>> +
>> +	/*
>> +	 * FIXME move the cdclk caclulation to
>> +	 * compute_config() so we can fail gracefully.
>> +	 */
>> +	if (cdclk > dev_priv->max_cdclk_freq) {
>> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
>> +			  cdclk, dev_priv->max_cdclk_freq);
>> +		cdclk = dev_priv->max_cdclk_freq;
>> +	}
>> +
>> +	to_intel_atomic_state(state)->cdclk = cdclk;
>> +
>> +	return 0;
>> +}
>> +
>> +static void skl_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>> +{
>> +	struct drm_device *dev = old_state->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	unsigned int req_cdclk = to_intel_atomic_state(old_state)->cdclk;
>> +
>> +	skl_set_cdclk(dev_priv, req_cdclk);
>> +}
>> +
>>   static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>>   				      struct intel_crtc_state *crtc_state)
>>   {
>> @@ -14831,6 +14894,11 @@ static void intel_init_display(struct drm_device *dev)
>>   			broxton_modeset_commit_cdclk;
>>   		dev_priv->display.modeset_calc_cdclk =
>>   			broxton_modeset_calc_cdclk;
>> +	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
>> +		dev_priv->display.modeset_commit_cdclk =
>> +			skl_modeset_commit_cdclk;
>> +		dev_priv->display.modeset_calc_cdclk =
>> +			skl_modeset_calc_cdclk;
>>   	}
>>
>>   	switch (INTEL_INFO(dev)->gen) {
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> 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

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

* Re: [PATCH] drm/i915/skl: CDCLK change during modeset based on VCO in use.
  2015-11-20 18:10   ` Clint Taylor
@ 2015-11-20 18:47     ` Ville Syrjälä
  0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2015-11-20 18:47 UTC (permalink / raw)
  To: Clint Taylor; +Cc: Shobhit Kumar, Intel-gfx

On Fri, Nov 20, 2015 at 10:10:43AM -0800, Clint Taylor wrote:
> On 11/20/2015 05:55 AM, Ville Syrjälä wrote:
> > On Thu, Nov 19, 2015 at 09:20:16AM -0800, clinton.a.taylor@intel.com wrote:
> >> From: Clint Taylor <clinton.a.taylor@intel.com>
> >>
> >> Add SKL and KBL cdclk changes during modeset. Taking into account new
> >> linkrates available using 8640 VCO.
> >>
> >> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_display.c |   68 ++++++++++++++++++++++++++++++++++
> >>   1 file changed, 68 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 2d7ea95..bed03cb 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -9727,6 +9727,69 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> >>   	broadwell_set_cdclk(dev, req_cdclk);
> >>   }
> >>
> >> +static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
> >> +{
> >> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> >> +	int max_pixclk = ilk_max_pixel_rate(state);
> >> +	int cdclk;
> >> +	uint32_t linkrate;
> >> +
> >> +	linkrate = (I915_READ(DPLL_CTRL1) &
> >> +		    DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
> >
> > I don't think we should read this from the hardware here. Instead
> > we should stash the proper vco somewhere under dev_priv. Well, we
> 
> I have no problem stuffing the proper VCO based on the link rate 
> determined in skl_edp_set_pll_config(). I think just storing a uint32 of 
> the required rate would be enough as we know 216000 and 432000 require 
> the 8640 VCO.
> 
> static const int skl_rates[] = { 162000, 216000, 270000,
> 				  324000, 432000, 540000 };
> 
> 
> > already have the boot_cdclk there, which is more or less just that.
> > What's really missing is code to fix up our initial boot_cdclk
> > choice if it turns out to be wrong. Where would it get fixed? I
> > assume we'd do that during/after eDP probing since then we should
> > know what link rate we want to use.
> 
> boot_cdclk is irrelevant as soon as boot has completed.

No. It's the thing we want to look at to see what we want the DPLL0
VCO to be. We should just rename it to something better, and probably
just store the VCO there instead of the cdclk.

> I agree we need 
> to fixup cdclk in cases when incorrect like at boot or during resume. 
> Both cases are outside the scope of this patch which is just to fix 
> cdclk during modeset when the boot_cdclk is < required CDCLK after an 
> HPD event.

I'm not thinking about fixing anything explicitly during resume/init.
We will do a modeset during resume, and the normal cdclk programming
will happen there. So there should be nothing more to do. And the
same thing will happen during boot in case the current cdclk is not
what we actually want.

So we just need to figure out is which VCO should we use. And for
that we need to know the eDP link frequency. Oh and we also need
to update max_cdclk to match in case it was originally determined
using the wrong VCO.

> 
> >
> >> +
> >> +	/*
> >> +	* FIXME should also account for plane ratio
> >> +	* once 64bpp pixel formats are supported.
> >> +	*/
> >> +
> >> +	if (linkrate == DPLL_CTRL1_LINK_RATE_2160 ||
> >> +	    linkrate == DPLL_CTRL1_LINK_RATE_1080) {
> >> +		/* vco 8640 */
> >> +		if (max_pixclk > 540000)
> >> +			cdclk = 617140;
> >> +		else if (max_pixclk > 432000)
> >> +			cdclk = 540000;
> >> +		else if (max_pixclk > 308570)
> >> +			cdclk = 432000;
> >> +		else
> >> +			cdclk = 308570;
> >> +	}
> >> +	else {
> >> +		/* VCO 8100 */
> >> +		if (max_pixclk > 540000)
> >> +			cdclk = 675000;
> >> +		else if (max_pixclk > 450000)
> >> +			cdclk = 540000;
> >> +		else if (max_pixclk > 337500)
> >> +			cdclk = 450000;
> >> +		else
> >> +			cdclk = 337500;
> >> +	}
> >> +
> >> +	/*
> >> +	 * FIXME move the cdclk caclulation to
> >> +	 * compute_config() so we can fail gracefully.
> >> +	 */
> >> +	if (cdclk > dev_priv->max_cdclk_freq) {
> >> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> >> +			  cdclk, dev_priv->max_cdclk_freq);
> >> +		cdclk = dev_priv->max_cdclk_freq;
> >> +	}
> >> +
> >> +	to_intel_atomic_state(state)->cdclk = cdclk;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void skl_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> >> +{
> >> +	struct drm_device *dev = old_state->dev;
> >> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> +	unsigned int req_cdclk = to_intel_atomic_state(old_state)->cdclk;
> >> +
> >> +	skl_set_cdclk(dev_priv, req_cdclk);
> >> +}
> >> +
> >>   static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> >>   				      struct intel_crtc_state *crtc_state)
> >>   {
> >> @@ -14831,6 +14894,11 @@ static void intel_init_display(struct drm_device *dev)
> >>   			broxton_modeset_commit_cdclk;
> >>   		dev_priv->display.modeset_calc_cdclk =
> >>   			broxton_modeset_calc_cdclk;
> >> +	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
> >> +		dev_priv->display.modeset_commit_cdclk =
> >> +			skl_modeset_commit_cdclk;
> >> +		dev_priv->display.modeset_calc_cdclk =
> >> +			skl_modeset_calc_cdclk;
> >>   	}
> >>
> >>   	switch (INTEL_INFO(dev)->gen) {
> >> --
> >> 1.7.9.5
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >

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

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

* Re: [PATCH] drm/i915/skl: CDCLK change during modeset based on VCO in use.
  2015-11-20 13:55 ` Ville Syrjälä
  2015-11-20 18:10   ` Clint Taylor
@ 2015-11-20 19:03   ` Daniel Stone
  2015-11-20 19:07     ` Ville Syrjälä
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel Stone @ 2015-11-20 19:03 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Shobhit Kumar, intel-gfx

On 20 November 2015 at 13:55, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Nov 19, 2015 at 09:20:16AM -0800, clinton.a.taylor@intel.com wrote:
>> +static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
>> +{
>> +     struct drm_i915_private *dev_priv = to_i915(state->dev);
>> +     int max_pixclk = ilk_max_pixel_rate(state);
>> +     int cdclk;
>> +     uint32_t linkrate;
>> +
>> +     linkrate = (I915_READ(DPLL_CTRL1) &
>> +                 DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
>
> I don't think we should read this from the hardware here. Instead
> we should stash the proper vco somewhere under dev_priv.

In state rather than dev_priv, surely?

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

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

* Re: [PATCH] drm/i915/skl: CDCLK change during modeset based on VCO in use.
  2015-11-20 19:03   ` Daniel Stone
@ 2015-11-20 19:07     ` Ville Syrjälä
  0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2015-11-20 19:07 UTC (permalink / raw)
  To: Daniel Stone; +Cc: Shobhit Kumar, intel-gfx

On Fri, Nov 20, 2015 at 07:03:28PM +0000, Daniel Stone wrote:
> On 20 November 2015 at 13:55, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Nov 19, 2015 at 09:20:16AM -0800, clinton.a.taylor@intel.com wrote:
> >> +static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
> >> +{
> >> +     struct drm_i915_private *dev_priv = to_i915(state->dev);
> >> +     int max_pixclk = ilk_max_pixel_rate(state);
> >> +     int cdclk;
> >> +     uint32_t linkrate;
> >> +
> >> +     linkrate = (I915_READ(DPLL_CTRL1) &
> >> +                 DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
> >
> > I don't think we should read this from the hardware here. Instead
> > we should stash the proper vco somewhere under dev_priv.
> 
> In state rather than dev_priv, surely?

No, it's going to be a fixed value, so doesn't need to be recomputed
ever.

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

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

end of thread, other threads:[~2015-11-20 19:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-19 17:20 [PATCH] drm/i915/skl: CDCLK change during modeset based on VCO in use clinton.a.taylor
2015-11-20 13:55 ` Ville Syrjälä
2015-11-20 18:10   ` Clint Taylor
2015-11-20 18:47     ` Ville Syrjälä
2015-11-20 19:03   ` Daniel Stone
2015-11-20 19:07     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox