Re: [PATCH 01/15] drm/i915: only disable DDI sound if intel_crtc->eld_vld

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Mar 07, 2013 at 11:31:23AM +0200, Ville Syrjälä wrote:
> On Wed, Mar 06, 2013 at 08:03:08PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > We already have the same check on intel_enable_ddi. This patch
> > prevents "unclaimed register" messages when the power well is
> > disabled.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c |    9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 56bb7cb..cd2f519 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1347,9 +1347,12 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
> >  		ironlake_edp_backlight_off(intel_dp);
> >  	}
> >  
> > -	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> > -	tmp &= ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) << (pipe * 4));
> > -	I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> > +	if (intel_crtc->eld_vld) {
> > +		tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> > +		tmp &= ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) <<
> > +			 (pipe * 4));
> > +		I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> > +	}
> 
> We set eld_vld=false before disabling the crtc in intel_crtc_disable().
> I think you need to rearrange that so that we clear eld_vld only
> after ->crtc_disable has been called.

Just looked a bit through the code here and ->eld_vld is another case of
where the ddi encoder needs to know something which only the connector
really knows currently. Since in our dp/hdmi/sdvo code we just check
<connector>->has_audio in the respective modeset function.

And it's not really the only place where the apparently common ddi
functions are just if ladders gropping around in connector details.

I guess we need to eventually clean this mess up, once things have settled
a bit, since I fear the duplication it icky little bugs this fragmentation
of state keeping might (or probably will) cause.

Ideas welcome ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch