From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 01/15] drm/i915: only disable DDI sound if intel_crtc->eld_vld Date: Sun, 17 Mar 2013 21:17:16 +0100 Message-ID: <20130317201716.GI9021@phenom.ffwll.local> References: <1362611003-4823-1-git-send-email-przanoni@gmail.com> <1362611003-4823-2-git-send-email-przanoni@gmail.com> <20130307093123.GQ4469@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ea0-f181.google.com (mail-ea0-f181.google.com [209.85.215.181]) by gabe.freedesktop.org (Postfix) with ESMTP id BA797E5C78 for ; Sun, 17 Mar 2013 13:14:31 -0700 (PDT) Received: by mail-ea0-f181.google.com with SMTP id z10so2183853ead.26 for ; Sun, 17 Mar 2013 13:14:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130307093123.GQ4469@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Thu, Mar 07, 2013 at 11:31:23AM +0200, Ville Syrj=E4l=E4 wrote: > On Wed, Mar 06, 2013 at 08:03:08PM -0300, Paulo Zanoni wrote: > > From: Paulo Zanoni > > = > > 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 > > --- > > 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/in= tel_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_encod= er *intel_encoder) > > ironlake_edp_backlight_off(intel_dp); > > } > > = > > - tmp =3D I915_READ(HSW_AUD_PIN_ELD_CP_VLD); > > - tmp &=3D ~((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 =3D I915_READ(HSW_AUD_PIN_ELD_CP_VLD); > > + tmp &=3D ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) << > > + (pipe * 4)); > > + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp); > > + } > = > We set eld_vld=3Dfalse 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 ->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