[Intel-gfx] [PATCH v3 04/11] drm/i915: Handle adjust better in intel_pipe_config_compare

[Daniel Vetter]

On Mon, Nov 13, 2017 at 07:24:22PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 10, 2017 at 02:02:45PM +0100, Daniel Vetter wrote:
> > On Fri, Nov 10, 2017 at 12:34:56PM +0100, Maarten Lankhorst wrote:
> > > Some parameters use CHECK_BOOLL, but should really use
> > > CHECK_BOOL_INCOMPLETE. We cannot currently check whether
> > > the inherited infoframes and audio are set up correctly.
> > > 
> > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++--
> > >  1 file changed, 17 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 425167da560b..f5933b0719c9 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11076,6 +11076,9 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
> > >  			  bool adjust)
> > >  {
> > >  	bool ret = true;
> > > +	bool fixup_inherited = adjust &&
> > > +		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
> > > +		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
> > >  
> > >  #define PIPE_CONF_CHECK_X(name)	\
> > >  	if (current_config->name != pipe_config->name) { \
> > > @@ -11104,6 +11107,17 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
> > >  		ret = false; \
> > >  	}
> > >  
> > 
> > Maybe add a comment here like
> > 
> > /*
> >  * Checks state where we only read out the enabling, but not the entire
> >  * state itself (like full infoframes or ELD for audio). These states
> >  * require a full modeset on bootup to fix up.
> 
> has_audio shouldn't need the ELD. If audio is enabled then it's enabled,
> otherwise it's not.
> 
> So I guess if the BIOS doesn't enable audio, then what we really need is
> a way to enable it without a full modeset. And I can't immediately
> recall if we can do that or not.

Afaiui the audio enable bit fires off the irq on the snd driver, which
then reads the eld to figure out how to set up the audio side.

So we indeed need the eld to be correct. We might also need more stuff
(maybe audio only reads eld when the irq fires, I didn't check).
 
> I'm worried that ignoring the issue will just lead to piles of bugs
> where people complain that their audio doesn't work.

I'm not blocking your patches, the r-b was included ... what do you mean?
I'm all for fixing this ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch