[Intel-gfx] [PATCH] drm/i915: Check for fused or unused pipes

[Jani Nikula]

On Fri, 15 Dec 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan at gmail.com> wrote:
> On Friday, December 15, 2017 11:27:02 AM PST Jani Nikula wrote:
>> On Fri, 15 Dec 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan at 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 at linutronix.de>
>> >> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> >> Signed-off-by: Mika Kahola <mika.kahola at 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.
>> 
>
> My concern was that we don't expose any interface to tell the audio driver how 
> many pipes are present, so a WARN_ON would be unreasonable. But on second 
> thoughts, audio should have ways to find that information from the hardware.

Yeah. And if it gets that wrong, we'd like to know. It just doesn't know
about fused stuff, which is why checking against num_pipes fails.

BR,
Jani.


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