Re: [PATCH] drm/i915: Check for fused or unused pipes

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

On Fri, 15 Dec 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com> wrote:
> On Friday, December 15, 2017 9:59:02 AM PST Mika Kahola wrote:
>> In case of fused or unused pipes, return early with a warning when reading
>> information for encoder.
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103206
>> Reported-by: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_audio.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> b/drivers/gpu/drm/i915/intel_audio.c index f1502a0..522d54f 100644
>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> @@ -779,7 +779,7 @@ static struct intel_encoder *get_saved_enc(struct
>> drm_i915_private *dev_priv, {
>>  	struct intel_encoder *encoder;
>> 
>> -	if (WARN_ON(pipe >= INTEL_INFO(dev_priv)->num_pipes))
>> +	if (WARN_ON(pipe >= ARRAY_SIZE(dev_priv->av_enc_map)))
>>  		return NULL;
>
> I think we should remove the WARN_ON() and just return NULL. The error return 
> and the debug message we print in the caller is good enough IMO. 

On the contrary, I think this one is fine with WARN_ON. The commit
message is inaccurate. This will only become a bounds check with the
change, and I think it's important to catch the totally out of bounds
values loudly.

In case of fused or unused pipes, the code will happily pass that check,
and check the av_enc_map for whether the pipe is available or not.

BR,
Jani.


>
>> 
>>  	/* MST */
>
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx