diff mbox

[v2,12/14] drm/i915: Remove the link rate and lane count loop in compute config

Message ID 1473364931-5657-1-git-send-email-manasi.d.navare@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Navare, Manasi Sept. 8, 2016, 8:02 p.m. UTC
While configuring the pipe during modeset, it should use
max clock and max lane count and reduce the bpp until
the requested mode rate is less than or equal to
available link BW.
This is required to pass DP Compliance.

v2:
* Removed the loop since we use max values of clock
and lane count (Dhinakaran Pandiyan)

Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

Comments

Dhinakaran Pandiyan Sept. 13, 2016, 1:14 a.m. UTC | #1
On Thu, 2016-09-08 at 13:02 -0700, Manasi Navare wrote:
> While configuring the pipe during modeset, it should use

> max clock and max lane count and reduce the bpp until

> the requested mode rate is less than or equal to

> available link BW.

> This is required to pass DP Compliance.

> 

> v2:

> * Removed the loop since we use max values of clock

> and lane count (Dhinakaran Pandiyan)

> 

> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>

> ---

>  drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++--------------

>  1 file changed, 8 insertions(+), 14 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c

> index 1378116..60c8857 100644

> --- a/drivers/gpu/drm/i915/intel_dp.c

> +++ b/drivers/gpu/drm/i915/intel_dp.c

> @@ -1567,20 +1567,14 @@ intel_dp_compute_config(struct intel_encoder *encoder,

>  	for (; bpp >= 6*3; bpp -= 2*3) {

>  		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,

>  						   bpp);

> -

> -		for (clock = min_clock; clock <= max_clock; clock++) {

> -			for (lane_count = min_lane_count;

> -				lane_count <= max_lane_count;

> -				lane_count <<= 1) {

> -

> -				link_clock = common_rates[clock];

> -				link_avail = intel_dp_max_data_rate(link_clock,

> -								    lane_count);

> -

> -				if (mode_rate <= link_avail) {

> -					goto found;

> -				}

> -			}

> +		clock = max_clock;

> +		lane_count = max_lane_count;


Do we still need lane_count? We can eliminate it if it's always going to
be max_lane_count. 


> +		link_clock = common_rates[clock];


Same here for link_clock.

> +		link_avail = intel_dp_max_data_rate(link_clock,

> +						    lane_count);

> +

> +		if (mode_rate <= link_avail) {

> +			goto found;


Print KMS debug if we cannot satisfy the mode_rate at the max DP link
rate?

>  		}

>  	}

>
Navare, Manasi Sept. 14, 2016, 1:05 a.m. UTC | #2
On Mon, Sep 12, 2016 at 06:14:00PM -0700, Pandiyan, Dhinakaran wrote:
> On Thu, 2016-09-08 at 13:02 -0700, Manasi Navare wrote:
> > While configuring the pipe during modeset, it should use
> > max clock and max lane count and reduce the bpp until
> > the requested mode rate is less than or equal to
> > available link BW.
> > This is required to pass DP Compliance.
> > 
> > v2:
> > * Removed the loop since we use max values of clock
> > and lane count (Dhinakaran Pandiyan)
> > 
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++--------------
> >  1 file changed, 8 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 1378116..60c8857 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1567,20 +1567,14 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  	for (; bpp >= 6*3; bpp -= 2*3) {
> >  		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
> >  						   bpp);
> > -
> > -		for (clock = min_clock; clock <= max_clock; clock++) {
> > -			for (lane_count = min_lane_count;
> > -				lane_count <= max_lane_count;
> > -				lane_count <<= 1) {
> > -
> > -				link_clock = common_rates[clock];
> > -				link_avail = intel_dp_max_data_rate(link_clock,
> > -								    lane_count);
> > -
> > -				if (mode_rate <= link_avail) {
> > -					goto found;
> > -				}
> > -			}
> > +		clock = max_clock;
> > +		lane_count = max_lane_count;
> 
> Do we still need lane_count? We can eliminate it if it's always going to
> be max_lane_count. 
> 
>

This is just to follow the paradigm and use lane count and link rate here
while setting up the pipe config.

 
> > +		link_clock = common_rates[clock];
> 
> Same here for link_clock.
> 
> > +		link_avail = intel_dp_max_data_rate(link_clock,
> > +						    lane_count);
> > +
> > +		if (mode_rate <= link_avail) {
> > +			goto found;
> 
> Print KMS debug if we cannot satisfy the mode_rate at the max DP link
> rate?
>

Agree, but I am going to add it outside the bpp loop. You should see 
a new patch soon.

Manasi 
> >  		}
> >  	}
> >  
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1378116..60c8857 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1567,20 +1567,14 @@  intel_dp_compute_config(struct intel_encoder *encoder,
 	for (; bpp >= 6*3; bpp -= 2*3) {
 		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
 						   bpp);
-
-		for (clock = min_clock; clock <= max_clock; clock++) {
-			for (lane_count = min_lane_count;
-				lane_count <= max_lane_count;
-				lane_count <<= 1) {
-
-				link_clock = common_rates[clock];
-				link_avail = intel_dp_max_data_rate(link_clock,
-								    lane_count);
-
-				if (mode_rate <= link_avail) {
-					goto found;
-				}
-			}
+		clock = max_clock;
+		lane_count = max_lane_count;
+		link_clock = common_rates[clock];
+		link_avail = intel_dp_max_data_rate(link_clock,
+						    lane_count);
+
+		if (mode_rate <= link_avail) {
+			goto found;
 		}
 	}