From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 4/4] drm/i915: move infoframe setting to after port enable Date: Thu, 3 Apr 2014 14:00:53 -0700 Message-ID: <20140403140053.3bd84fa2@jbarnes-desktop> References: <1396458534-23108-1-git-send-email-jbarnes@virtuousgeek.org> <1396458534-23108-4-git-send-email-jbarnes@virtuousgeek.org> <20140403151956.GG7225@phenom.ffwll.local> <20140403094915.2aea8c8f@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from gproxy1-pub.mail.unifiedlayer.com (gproxy1-pub.mail.unifiedlayer.com [69.89.25.95]) by gabe.freedesktop.org (Postfix) with SMTP id 26AE46EA4C for ; Thu, 3 Apr 2014 13:59:45 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx List-Id: intel-gfx@lists.freedesktop.org On Thu, 3 Apr 2014 22:55:24 +0200 Daniel Vetter wrote: > On Thu, Apr 3, 2014 at 6:49 PM, Jesse Barnes wrote: > >> > static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder, > >> > @@ -738,9 +736,13 @@ static void intel_enable_hdmi(struct intel_encoder *encoder) > >> > struct drm_i915_private *dev_priv = dev->dev_private; > >> > struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc); > >> > struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > >> > + struct drm_display_mode *adjusted_mode = > >> > + &intel_crtc->config.adjusted_mode; > >> > u32 temp; > >> > u32 enable_bits = SDVO_ENABLE; > >> > > >> > + intel_hdmi->set_infoframes(&encoder->base, adjusted_mode); > >> > + > >> > if (intel_hdmi->has_audio) > >> > enable_bits |= SDVO_AUDIO_ENABLE; > >> > >> That kind of change tends to freak out Paulo, our master of infoframes. Do > >> doecs really state that this is how stuff should work in general, or is > >> this just a gm45/vlv thing? Or vlv only? > >> > >> /me remembers how often we've burnt our hands here > > > > Hey infoframe emission was totally broken for awhile due to a generic > > change, and we didn't notice that right away. :) > > > > But yeah I'd prefer to test this on multiple platforms first, but don't > > have that capability. It does pass on BYT though, and the logic should > > be similar to IBX, so this change ought to be safe. It's easy to > > revert too and make platform specific if we get regression reports, but > > I expect it to fix weird issues instead of introducing new ones, based > > on the infoframe analyzer results we have from BYT. > > Yeah I guess the number of users who actual use a gm45 or so with a TV > is probably still bigger than all the byt platforms out there :( If > Paulo can ack this I'll happily merge. Paulo, can you please take a > quick look? > > Also you make this sound like it's a regression, but the patch is > missing cc:stable and a sha1 citation of the offending commit. Jesse, > can you please fix this? No it's not a regression, we had an earlier regression on infoframes though that seemed to have gone unnoticed for awhile when looking at the logs and testing here... It's fixed now though. -- Jesse Barnes, Intel Open Source Technology Center