[Intel-gfx] [PATCH v1½ 12/13] drm/i915/dp: localize link rate index variable more

[Jani Nikula]

On Thu, 02 Feb 2017, Manasi Navare <manasi.d.navare at intel.com> wrote:
> On Thu, Feb 02, 2017 at 10:42:48AM +0200, Jani Nikula wrote:
>> On Thu, 02 Feb 2017, Manasi Navare <manasi.d.navare at intel.com> wrote:
>> > On Thu, Jan 26, 2017 at 09:44:26PM +0200, Jani Nikula wrote:
>> >> Localize link_rate_index to the if block, and rename to just index to
>> >> reduce indent.
>> >> 
>> >> Cc: Manasi Navare <manasi.d.navare at intel.com>
>> >> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> >> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
>> >>  1 file changed, 7 insertions(+), 6 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> >> index 7704d32286a3..429dc70c251a 100644
>> >> --- a/drivers/gpu/drm/i915/intel_dp.c
>> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> >> @@ -1639,7 +1639,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>> >>  	/* Conveniently, the link BW constants become indices with a shift...*/
>> >>  	int min_clock = 0;
>> >>  	int max_clock;
>> >> -	int link_rate_index;
>> >>  	int bpp, mode_rate;
>> >>  	int link_avail, link_clock;
>> >>  	const int *common_rates = intel_dp->common_rates;
>> >> @@ -1684,11 +1683,13 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>> >>  
>> >>  	/* Use values requested by Compliance Test Request */
>> >>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
>> >> -		link_rate_index = intel_dp_find_rate(intel_dp->common_rates,
>> >> -						     intel_dp->num_common_rates,
>> >> -						     intel_dp->compliance.test_link_rate);
>> >
>> > Can we not pass just the common_rates as an argument to this function
>> > since common_rates is already assigned intel_dp->common_rates.
>> 
>> Do you mean just pass intel_dp to intel_dp_find_rate? If yes, I think
>> this keeps intel_dp_find_rate generic, independent of intel_dp or
>> anything else, and you can be sure it's stateless (same as
>> intersect_rates).
>> 
>> If you don't mean that, I don't know what you mean... please explain.
>> 
>> BR,
>> Jani.
>> 
>>
>
> I agree that it needs to be stateless and hence we dont pass intel_dp directly.
> What I was saying was that at the beginning of the function we do initialize
> common_rates = intel_dp->common_rates;
> so why cant we pass just common_rates to this function instead of intel_dp->common_rates?
> Not sure if this optimization will make any big impact but just a thought.

Due to the function already being so big and having so many local
variables, I ended up throwing the local common_rates away altogether,
and changing everything in the function to use intel_dp->common_rates.

BR,
Jani.

>
> Regards
> Manasi 
>> >
>> > Regards
>> > Manasi
>> >
>> >
>> >> -		if (link_rate_index >= 0)
>> >> -			min_clock = max_clock = link_rate_index;
>> >> +		int index;
>> >> +
>> >> +		index = intel_dp_find_rate(intel_dp->common_rates,
>> >> +					   intel_dp->num_common_rates,
>> >> +					   intel_dp->compliance.test_link_rate);
>> >> +		if (index >= 0)
>> >> +			min_clock = max_clock = index;
>> >>  		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
>> >>  	}
>> >>  	DRM_DEBUG_KMS("DP link computation with max lane count %i "
>> >> -- 
>> >> 2.1.4
>> >> 
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center