[Intel-gfx] [PATCH v3 2/6] drm/i915: Remove the link rate and lane count loop in compute config

[Manasi Navare]

On Wed, Sep 28, 2016 at 10:14:45AM +0300, Jani Nikula wrote:
> On Wed, 28 Sep 2016, Manasi Navare <manasi.d.navare at intel.com> wrote:
> > On Tue, Sep 27, 2016 at 04:39:38PM +0300, Jani Nikula wrote:
> >> On Mon, 26 Sep 2016, Jani Nikula <jani.nikula at linux.intel.com> wrote:
> >> > On Fri, 16 Sep 2016, Manasi Navare <manasi.d.navare at intel.com> 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.
> >> >
> >> > As I wrote in reply to patch 1/6, this is not a DP spec requirement. The
> >> > link policy maker can freely choose the link parameters as long as the
> >> > sink supports them.
> >> 
> >> Also double checked the DP link CTS spec. AFAICT none of the tests
> >> expect the source to use the max clock or max lane count
> >> directly. (Automated test request is another matter, and we should look
> >> at it.)
> >> 
> >> I think patches 1-2 are based on an incorrect interpretation of the spec
> >> and tests.
> >> 
> >> BR,
> >> Jani.
> >>
> >
> > I have the patches for handling the automated test request from DPR for
> > compliance testing as mentioned in the CTS spec. But they have dependencies
> > on these patches (1-6) so I will submit them after these get merged.
> 
> We need to re-evaluate the ordering of the patches.
> 
> BR,
> Jani.
>

Thanks for your feedback.
I am reevaluating the need for this patch based on your recommendation.
I am currently testing it with keeping the optimal values of the link rate
here and only train at the max link rate when we recieve the test request
from DPR 120. 
In this case we would need to configure the PLLs base don the requested target
link rate and train the link at that rate and after the test is done restore
the original state of the PLLs. Does that sound like a good approach, is this
what you were suggesting?

Regards
Manasi


> >
> > Regards
> > Manasi
> >  
> >> 
> >> >
> >> > BR,
> >> > Jani.
> >> >
> >> >
> >> >>
> >> >> v3:
> >> >> * Add Debug print if requested mode cannot be supported
> >> >> during modeset (Dhinakaran Pandiyan)
> >> >> v2:
> >> >> * Removed the loop since we use max values of clock
> >> >> and lane count (Dhinakaran Pandiyan)
> >> >>
> >> >> Signed-off-by: Manasi Navare <manasi.d.navare at 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 d81c67cb..65b4559 100644
> >> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> >> @@ -1644,23 +1644,17 @@ 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);
> >> >> +		clock = max_clock;
> >> >> +		lane_count = max_lane_count;
> >> >> +		link_clock = common_rates[clock];
> >> >> +		link_avail = intel_dp_max_data_rate(link_clock,
> >> >> +						    lane_count);
> >> >>  
> >> >> -		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;
> >> >> -				}
> >> >> -			}
> >> >> -		}
> >> >> +		if (mode_rate <= link_avail)
> >> >> +			goto found;
> >> >>  	}
> >> >>  
> >> >> +	DRM_DEBUG_KMS("Requested Mode Rate not supported\n");
> >> >>  	return false;
> >> >>  
> >> >>  found:
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Technology Center
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center