[Intel-gfx] [PATCH v3 02/14] drm/i915/dp: return errors from rate_to_index()

[Jani Nikula]

On Tue, 28 Mar 2017, Manasi Navare <manasi.d.navare at intel.com> wrote:
> Won't it make more sense to squash this patch with Patch 01 in this series?
> When i was reading Patch 1, I almost was going to comment about
> handling the case where we dont find the index..

Matter of taste. You might then look at patch 3 and figure it should be
squashed too... you have to draw the line somewhere, and I think it's
always easier to review smaller pieces and be sure they are each
correct. If you find an issue with the 2nd patch, the 1st one is still
valid, and the follow-up review is also easier.

BR,
Jani.


>
> Regards
> Manasi
>
> On Tue, Mar 28, 2017 at 05:59:02PM +0300, Jani Nikula wrote:
>> We shouldn't silently use the first element if we can't find the rate
>> we're looking for. Make rate_to_index() more generally useful, and
>> fallback to the first element in the caller, with a big warning.
>> 
>> 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, 9 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 88c708b07c70..0e200a37b75b 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1544,9 +1544,9 @@ static int rate_to_index(const int *rates, int len, int rate)
>>  
>>  	for (i = 0; i < len; i++)
>>  		if (rate == rates[i])
>> -			break;
>> +			return i;
>>  
>> -	return i;
>> +	return -1;
>>  }
>>  
>>  int
>> @@ -1564,8 +1564,13 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
>>  
>>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
>>  {
>> -	return rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
>> -			     rate);
>> +	int i = rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
>> +			      rate);
>> +
>> +	if (WARN_ON(i < 0))
>> +		i = 0;
>> +
>> +	return i;
>>  }
>>  
>>  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>> -- 
>> 2.1.4
>> 

-- 
Jani Nikula, Intel Open Source Technology Center