Re: [Intel-gfx] [PATCH v2 2/5] drm/i915/dpll: replace BUG_ON() with drm_WARN_ON()

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 31 Aug 2022, Andrzej Hajda <andrzej.hajda@intel.com> wrote:
> On 30.08.2022 11:34, Jani Nikula wrote:
>> Avoid BUG_ON(). Actually check the dpll count and bail out loudly with
>> drm_WARN_ON() from the loop before overflowing
>> i915->dpll.shared_dplls[].
>> 
>> v2: Rebase
>> 
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> index bbe142056c7c..ed267c918009 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> @@ -4199,6 +4199,10 @@ void intel_shared_dpll_init(struct drm_i915_private *dev_priv)
>>   	dpll_info = dpll_mgr->dpll_info;
>>   
>>   	for (i = 0; dpll_info[i].name; i++) {
>> +		if (drm_WARN_ON(&dev_priv->drm,
>> +				i >= ARRAY_SIZE(dev_priv->display.dpll.shared_dplls)))
>> +			break;
>> +
>
> Shouldn't this check be replaced by some compile time checker.

Can't do that because the dpll_info arrays are null terminated.

> Anyway:
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Thanks, but I just pushed the series as Ville already reviewed it.

BR,
Jani.


>
> Regards
> Andrzej
>
>
>>   		drm_WARN_ON(&dev_priv->drm, i != dpll_info[i].id);
>>   		dev_priv->display.dpll.shared_dplls[i].info = &dpll_info[i];
>>   	}
>> @@ -4206,8 +4210,6 @@ void intel_shared_dpll_init(struct drm_i915_private *dev_priv)
>>   	dev_priv->display.dpll.mgr = dpll_mgr;
>>   	dev_priv->display.dpll.num_shared_dpll = i;
>>   	mutex_init(&dev_priv->display.dpll.lock);
>> -
>> -	BUG_ON(dev_priv->display.dpll.num_shared_dpll > I915_NUM_PLLS);
>>   }
>>   
>>   /**
>

-- 
Jani Nikula, Intel Open Source Graphics Center