public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Factor out p2 divider selection for pre-ilk platforms
@ 2015-06-18 10:47 ville.syrjala
  2015-06-18 15:08 ` Imre Deak
  0 siblings, 1 reply; 3+ messages in thread
From: ville.syrjala @ 2015-06-18 10:47 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The same dpll p2 divider selection is repeated three times in the
gen2-4 .find_dpll() functions. Factor it out.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 78 ++++++++++++++----------------------
 1 file changed, 30 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2fa81ed..2cc8ae7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -643,16 +643,12 @@ static bool intel_PLL_is_valid(struct drm_device *dev,
 	return true;
 }
 
-static bool
-i9xx_find_best_dpll(const intel_limit_t *limit,
-		    struct intel_crtc_state *crtc_state,
-		    int target, int refclk, intel_clock_t *match_clock,
-		    intel_clock_t *best_clock)
+static int
+i9xx_select_p2_div(const intel_limit_t *limit,
+		   const struct intel_crtc_state *crtc_state,
+		   int target)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_device *dev = crtc->base.dev;
-	intel_clock_t clock;
-	int err = target;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 
 	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		/*
@@ -661,18 +657,31 @@ i9xx_find_best_dpll(const intel_limit_t *limit,
 		 * single/dual channel state, if we even can.
 		 */
 		if (intel_is_dual_link_lvds(dev))
-			clock.p2 = limit->p2.p2_fast;
+			return limit->p2.p2_fast;
 		else
-			clock.p2 = limit->p2.p2_slow;
+			return limit->p2.p2_slow;
 	} else {
 		if (target < limit->p2.dot_limit)
-			clock.p2 = limit->p2.p2_slow;
+			return limit->p2.p2_slow;
 		else
-			clock.p2 = limit->p2.p2_fast;
+			return limit->p2.p2_fast;
 	}
+}
+
+static bool
+i9xx_find_best_dpll(const intel_limit_t *limit,
+		    struct intel_crtc_state *crtc_state,
+		    int target, int refclk, intel_clock_t *match_clock,
+		    intel_clock_t *best_clock)
+{
+	struct drm_device *dev = crtc_state->base.crtc->dev;
+	intel_clock_t clock;
+	int err = target;
 
 	memset(best_clock, 0, sizeof(*best_clock));
 
+	clock.p2 = i9xx_select_p2_div(limit, crtc_state, target);
+
 	for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max;
 	     clock.m1++) {
 		for (clock.m2 = limit->m2.min;
@@ -712,30 +721,14 @@ pnv_find_best_dpll(const intel_limit_t *limit,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	intel_clock_t clock;
 	int err = target;
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
-		/*
-		 * For LVDS just rely on its current settings for dual-channel.
-		 * We haven't figured out how to reliably set up different
-		 * single/dual channel state, if we even can.
-		 */
-		if (intel_is_dual_link_lvds(dev))
-			clock.p2 = limit->p2.p2_fast;
-		else
-			clock.p2 = limit->p2.p2_slow;
-	} else {
-		if (target < limit->p2.dot_limit)
-			clock.p2 = limit->p2.p2_slow;
-		else
-			clock.p2 = limit->p2.p2_fast;
-	}
-
 	memset(best_clock, 0, sizeof(*best_clock));
 
+	clock.p2 = i9xx_select_p2_div(limit, crtc_state, target);
+
 	for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max;
 	     clock.m1++) {
 		for (clock.m2 = limit->m2.min;
@@ -773,28 +766,17 @@ g4x_find_best_dpll(const intel_limit_t *limit,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	intel_clock_t clock;
 	int max_n;
-	bool found;
+	bool found = false;
 	/* approximately equals target * 0.00585 */
 	int err_most = (target >> 8) + (target >> 9);
-	found = false;
-
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
-		if (intel_is_dual_link_lvds(dev))
-			clock.p2 = limit->p2.p2_fast;
-		else
-			clock.p2 = limit->p2.p2_slow;
-	} else {
-		if (target < limit->p2.dot_limit)
-			clock.p2 = limit->p2.p2_slow;
-		else
-			clock.p2 = limit->p2.p2_fast;
-	}
 
 	memset(best_clock, 0, sizeof(*best_clock));
+
+	clock.p2 = i9xx_select_p2_div(limit, crtc_state, target);
+
 	max_n = limit->n.max;
 	/* based on hardware requirement, prefer smaller n to precision */
 	for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) {
-- 
2.3.6

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Factor out p2 divider selection for pre-ilk platforms
  2015-06-18 10:47 [PATCH] drm/i915: Factor out p2 divider selection for pre-ilk platforms ville.syrjala
@ 2015-06-18 15:08 ` Imre Deak
  2015-06-22 13:45   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Imre Deak @ 2015-06-18 15:08 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On to, 2015-06-18 at 13:47 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The same dpll p2 divider selection is repeated three times in the
> gen2-4 .find_dpll() functions. Factor it out.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks ok to me:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 78 ++++++++++++++----------------------
>  1 file changed, 30 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2fa81ed..2cc8ae7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -643,16 +643,12 @@ static bool intel_PLL_is_valid(struct drm_device *dev,
>  	return true;
>  }
>  
> -static bool
> -i9xx_find_best_dpll(const intel_limit_t *limit,
> -		    struct intel_crtc_state *crtc_state,
> -		    int target, int refclk, intel_clock_t *match_clock,
> -		    intel_clock_t *best_clock)
> +static int
> +i9xx_select_p2_div(const intel_limit_t *limit,
> +		   const struct intel_crtc_state *crtc_state,
> +		   int target)
>  {
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> -	struct drm_device *dev = crtc->base.dev;
> -	intel_clock_t clock;
> -	int err = target;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  
>  	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		/*
> @@ -661,18 +657,31 @@ i9xx_find_best_dpll(const intel_limit_t *limit,
>  		 * single/dual channel state, if we even can.
>  		 */
>  		if (intel_is_dual_link_lvds(dev))
> -			clock.p2 = limit->p2.p2_fast;
> +			return limit->p2.p2_fast;
>  		else
> -			clock.p2 = limit->p2.p2_slow;
> +			return limit->p2.p2_slow;
>  	} else {
>  		if (target < limit->p2.dot_limit)
> -			clock.p2 = limit->p2.p2_slow;
> +			return limit->p2.p2_slow;
>  		else
> -			clock.p2 = limit->p2.p2_fast;
> +			return limit->p2.p2_fast;
>  	}
> +}
> +
> +static bool
> +i9xx_find_best_dpll(const intel_limit_t *limit,
> +		    struct intel_crtc_state *crtc_state,
> +		    int target, int refclk, intel_clock_t *match_clock,
> +		    intel_clock_t *best_clock)
> +{
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
> +	intel_clock_t clock;
> +	int err = target;
>  
>  	memset(best_clock, 0, sizeof(*best_clock));
>  
> +	clock.p2 = i9xx_select_p2_div(limit, crtc_state, target);
> +
>  	for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max;
>  	     clock.m1++) {
>  		for (clock.m2 = limit->m2.min;
> @@ -712,30 +721,14 @@ pnv_find_best_dpll(const intel_limit_t *limit,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	intel_clock_t clock;
>  	int err = target;
>  
> -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> -		/*
> -		 * For LVDS just rely on its current settings for dual-channel.
> -		 * We haven't figured out how to reliably set up different
> -		 * single/dual channel state, if we even can.
> -		 */
> -		if (intel_is_dual_link_lvds(dev))
> -			clock.p2 = limit->p2.p2_fast;
> -		else
> -			clock.p2 = limit->p2.p2_slow;
> -	} else {
> -		if (target < limit->p2.dot_limit)
> -			clock.p2 = limit->p2.p2_slow;
> -		else
> -			clock.p2 = limit->p2.p2_fast;
> -	}
> -
>  	memset(best_clock, 0, sizeof(*best_clock));
>  
> +	clock.p2 = i9xx_select_p2_div(limit, crtc_state, target);
> +
>  	for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max;
>  	     clock.m1++) {
>  		for (clock.m2 = limit->m2.min;
> @@ -773,28 +766,17 @@ g4x_find_best_dpll(const intel_limit_t *limit,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	intel_clock_t clock;
>  	int max_n;
> -	bool found;
> +	bool found = false;
>  	/* approximately equals target * 0.00585 */
>  	int err_most = (target >> 8) + (target >> 9);
> -	found = false;
> -
> -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> -		if (intel_is_dual_link_lvds(dev))
> -			clock.p2 = limit->p2.p2_fast;
> -		else
> -			clock.p2 = limit->p2.p2_slow;
> -	} else {
> -		if (target < limit->p2.dot_limit)
> -			clock.p2 = limit->p2.p2_slow;
> -		else
> -			clock.p2 = limit->p2.p2_fast;
> -	}
>  
>  	memset(best_clock, 0, sizeof(*best_clock));
> +
> +	clock.p2 = i9xx_select_p2_div(limit, crtc_state, target);
> +
>  	max_n = limit->n.max;
>  	/* based on hardware requirement, prefer smaller n to precision */
>  	for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) {


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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Factor out p2 divider selection for pre-ilk platforms
  2015-06-18 15:08 ` Imre Deak
@ 2015-06-22 13:45   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2015-06-22 13:45 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Thu, Jun 18, 2015 at 06:08:17PM +0300, Imre Deak wrote:
> On to, 2015-06-18 at 13:47 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The same dpll p2 divider selection is repeated three times in the
> > gen2-4 .find_dpll() functions. Factor it out.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Looks ok to me:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 78 ++++++++++++++----------------------
> >  1 file changed, 30 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 2fa81ed..2cc8ae7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -643,16 +643,12 @@ static bool intel_PLL_is_valid(struct drm_device *dev,
> >  	return true;
> >  }
> >  
> > -static bool
> > -i9xx_find_best_dpll(const intel_limit_t *limit,
> > -		    struct intel_crtc_state *crtc_state,
> > -		    int target, int refclk, intel_clock_t *match_clock,
> > -		    intel_clock_t *best_clock)
> > +static int
> > +i9xx_select_p2_div(const intel_limit_t *limit,
> > +		   const struct intel_crtc_state *crtc_state,
> > +		   int target)
> >  {
> > -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > -	struct drm_device *dev = crtc->base.dev;
> > -	intel_clock_t clock;
> > -	int err = target;
> > +	struct drm_device *dev = crtc_state->base.crtc->dev;
> >  
> >  	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> >  		/*
> > @@ -661,18 +657,31 @@ i9xx_find_best_dpll(const intel_limit_t *limit,
> >  		 * single/dual channel state, if we even can.
> >  		 */
> >  		if (intel_is_dual_link_lvds(dev))
> > -			clock.p2 = limit->p2.p2_fast;
> > +			return limit->p2.p2_fast;
> >  		else
> > -			clock.p2 = limit->p2.p2_slow;
> > +			return limit->p2.p2_slow;
> >  	} else {
> >  		if (target < limit->p2.dot_limit)
> > -			clock.p2 = limit->p2.p2_slow;
> > +			return limit->p2.p2_slow;
> >  		else
> > -			clock.p2 = limit->p2.p2_fast;
> > +			return limit->p2.p2_fast;
> >  	}
> > +}
> > +
> > +static bool
> > +i9xx_find_best_dpll(const intel_limit_t *limit,
> > +		    struct intel_crtc_state *crtc_state,
> > +		    int target, int refclk, intel_clock_t *match_clock,
> > +		    intel_clock_t *best_clock)
> > +{
> > +	struct drm_device *dev = crtc_state->base.crtc->dev;
> > +	intel_clock_t clock;
> > +	int err = target;
> >  
> >  	memset(best_clock, 0, sizeof(*best_clock));
> >  
> > +	clock.p2 = i9xx_select_p2_div(limit, crtc_state, target);
> > +
> >  	for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max;
> >  	     clock.m1++) {
> >  		for (clock.m2 = limit->m2.min;
> > @@ -712,30 +721,14 @@ pnv_find_best_dpll(const intel_limit_t *limit,
> >  		   int target, int refclk, intel_clock_t *match_clock,
> >  		   intel_clock_t *best_clock)
> >  {
> > -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > -	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_device *dev = crtc_state->base.crtc->dev;
> >  	intel_clock_t clock;
> >  	int err = target;
> >  
> > -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> > -		/*
> > -		 * For LVDS just rely on its current settings for dual-channel.
> > -		 * We haven't figured out how to reliably set up different
> > -		 * single/dual channel state, if we even can.
> > -		 */
> > -		if (intel_is_dual_link_lvds(dev))
> > -			clock.p2 = limit->p2.p2_fast;
> > -		else
> > -			clock.p2 = limit->p2.p2_slow;
> > -	} else {
> > -		if (target < limit->p2.dot_limit)
> > -			clock.p2 = limit->p2.p2_slow;
> > -		else
> > -			clock.p2 = limit->p2.p2_fast;
> > -	}
> > -
> >  	memset(best_clock, 0, sizeof(*best_clock));
> >  
> > +	clock.p2 = i9xx_select_p2_div(limit, crtc_state, target);
> > +
> >  	for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max;
> >  	     clock.m1++) {
> >  		for (clock.m2 = limit->m2.min;
> > @@ -773,28 +766,17 @@ g4x_find_best_dpll(const intel_limit_t *limit,
> >  		   int target, int refclk, intel_clock_t *match_clock,
> >  		   intel_clock_t *best_clock)
> >  {
> > -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > -	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_device *dev = crtc_state->base.crtc->dev;
> >  	intel_clock_t clock;
> >  	int max_n;
> > -	bool found;
> > +	bool found = false;
> >  	/* approximately equals target * 0.00585 */
> >  	int err_most = (target >> 8) + (target >> 9);
> > -	found = false;
> > -
> > -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> > -		if (intel_is_dual_link_lvds(dev))
> > -			clock.p2 = limit->p2.p2_fast;
> > -		else
> > -			clock.p2 = limit->p2.p2_slow;
> > -	} else {
> > -		if (target < limit->p2.dot_limit)
> > -			clock.p2 = limit->p2.p2_slow;
> > -		else
> > -			clock.p2 = limit->p2.p2_fast;
> > -	}
> >  
> >  	memset(best_clock, 0, sizeof(*best_clock));
> > +
> > +	clock.p2 = i9xx_select_p2_div(limit, crtc_state, target);
> > +
> >  	max_n = limit->n.max;
> >  	/* based on hardware requirement, prefer smaller n to precision */
> >  	for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) {
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-06-22 13:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-18 10:47 [PATCH] drm/i915: Factor out p2 divider selection for pre-ilk platforms ville.syrjala
2015-06-18 15:08 ` Imre Deak
2015-06-22 13:45   ` Daniel Vetter

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