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

[Jani Nikula]

On Wed, 28 Sep 2016, Manasi Navare <manasi.d.navare at intel.com> wrote:
> On Mon, Sep 26, 2016 at 04:41:27PM +0300, Jani Nikula 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.
>> 
>> BR,
>> Jani.
>> 
>>
>
> Thanks for your review feedback.
> This change was driven by Video Pattern generation tests in CTS spec. Eg: In
> test 4.3.3.1, the test requests 640x480 @ max link rate of 2.7Gbps and 4 lanes.
> The test will pass if it sets the link rate to 2.7 and lane count = 4.
> But in the existing implementation, this video mode request triggers a modeset
> but the compute_config function starts with the lowest link rate and lane count and
> trains the link at 1.62 and 4 lanes which does not match the expeced values of link
> rate = 2.7 and lane count = 4 and the test fails. 

Again, the test does not require us to use the maximum parameters by
default. It allows us to use optimal parameters by default, and use the
sink issued automated test request to change the link parameters to what
the test wants.

Look at the table in CTS 4.3.3.1. There's a test for 640x480 with 1.62
Gbps and 1 lane. And then there's a test for 640x480 with 2.7 Gbps and 4
lanes. What you're suggesting is to use excessive bandwidth for the mode
by default just because the test has been designed to be lax and allow
certain parameters at a minimum, instead of requiring optimal
parameters.

I do not think this is a change we want to make for DP SST, and it is
not a DP spec or compliance requirement.

BR,
Jani.


>
> Regards
> Manasi
>  
>> >
>> > 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