Re: [PATCH] drm/i915: Cache max number of pipes

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

On Mon, 16 Oct 2017, Mika Kahola <mika.kahola@intel.com> wrote:
> On Thu, 2017-10-12 at 09:55 +0300, Jani Nikula wrote:
>> On Thu, 12 Oct 2017, Mika Kahola <mika.kahola@intel.com> wrote:
>> > 
>> > CI system spotted an error on CFL system when running IGT tests
>> > with
>> > display disabled. In this case, the 'INTEL_INFO(dev_priv)-
>> > >num_pipes'
>> > is set to 0. This will cause that we prematurely return from
>> > 'get_saved_enc()'.
>> > 
>> > To fix this issue, the patch introduces a 'max_pipes' variable
>> > which caches
>> > the maximum number of available pipes.
>> This is not the right fix. The audio component init should be skipped
>> and thus all component calls blocked when num_pipes == 0. It's just
>> that
>> the num_pipes = 0 happens way too late when the display is fused off
>> or
>> disabled via modparam.
>> 
>> This makes me wonder how many things we actually screw up because
>> intel_device_info_runtime_init() happens too
> Right. So instead of doing this we shouldn't initialize the audio at
> all in case of disabled display? num_pipes variable just gets updated
> too late and audio part has already started to initialize itself.

Correct.

BR,
Jani.

>  
>>  late.
>> 
>> > 
>> > Reference: https://bugs.freedesktop.org/show_bug.cgi?id=103206
>> If this patch fixes the bug, we use Bugzilla: tag. Reference: is for
>> other references, e.g. the referenced bug is related to the patch at
>> hand somehow, but this does not fix the bug.
>> 
>> BR,
>> Jani.
>> 
>> 
>> > 
>> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.h          | 1 +
>> >  drivers/gpu/drm/i915/intel_audio.c       | 2 +-
>> >  drivers/gpu/drm/i915/intel_device_info.c | 2 ++
>> >  3 files changed, 4 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> > b/drivers/gpu/drm/i915/i915_drv.h
>> > index 770305b..6229ff8 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -868,6 +868,7 @@ struct intel_device_info {
>> >  	u8 num_pipes;
>> >  	u8 num_sprites[I915_MAX_PIPES];
>> >  	u8 num_scalers[I915_MAX_PIPES];
>> > +	u8 max_pipes;
>> >  
>> >  	unsigned int page_sizes; /* page sizes supported by the HW
>> > */
>> >  
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> > b/drivers/gpu/drm/i915/intel_audio.c
>> > index 0ddba16..147bd3d 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -754,7 +754,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 >= INTEL_INFO(dev_priv)->max_pipes))
>> >  		return NULL;
>> >  
>> >  	/* MST */
>> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
>> > b/drivers/gpu/drm/i915/intel_device_info.c
>> > index 875d428..ba1a807 100644
>> > --- a/drivers/gpu/drm/i915/intel_device_info.c
>> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> > @@ -376,6 +376,8 @@ void intel_device_info_runtime_init(struct
>> > drm_i915_private *dev_priv)
>> >  			info->num_sprites[pipe] = 1;
>> >  	}
>> >  
>> > +	info->max_pipes = info->num_pipes;
>> > +
>> >  	if (i915_modparams.disable_display) {
>> >  		DRM_INFO("Display disabled (module parameter)\n");
>> >  		info->num_pipes = 0;

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