All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Ander Conselvan De Oliveira" <conselvan2@gmail.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	drm-intel-fixes@lists.freedesktop.org,
	Libin Yang <libin.yang@linux.intel.com>
Subject: Re: [PATCH] drm/i915: Fix MST audio enabling
Date: Mon, 14 Mar 2016 08:40:51 +0200	[thread overview]
Message-ID: <878u1l8suk.fsf@intel.com> (raw)
In-Reply-To: <1457711542.2962.6.camel@gmail.com>

On Fri, 11 Mar 2016, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> [ text/plain ]
> On Fri, 2016-03-11 at 17:37 +0200, Ville Syrjälä wrote:
>> On Fri, Mar 11, 2016 at 05:33:06PM +0200, Ander Conselvan de Oliveira wrote:
>> > Commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") erroneously
>> > added a check for MST encoder to ilk_audio_codec_enable instead of the
>> > hsw+ equivalent. That causes the HDMI path to be used, resulting in a
>> > wrong register value.
>> > 
>> > Cc: Libin Yang <libin.yang@linux.intel.com>
>> > Cc: drm-intel-fixes@lists.freedesktop.org
>> > Fixes: commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio")
>> > Signed-off-by: Ander Conselvan de Oliveira <
>> > ander.conselvan.de.oliveira@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_audio.c | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> > b/drivers/gpu/drm/i915/intel_audio.c
>> > index 30f9214..f5fccf7 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -329,7 +329,8 @@ static void hsw_audio_codec_enable(struct drm_connector
>> > *connector,
>> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> >  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
>> > -	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
>> > +	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
>> > +	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
>> 
>> Humm. I thought had I asked all of these to be changed to has_dp_encoder.
>> I wonder if I imagined it or if I was just ignored.
>
> Ah, yes. Libin sent a patch to fix the same bug and you asked for that [1] and
> even R-b the follow up version even though it didn't use it everywhere. But it
> looks like Daniel didn't merge it since he wanted all uses converted to
> has_dp_encoders.

We want the minimal fix first now because by bikeshedding we let the bug
slip to v4.5, and we need cc: stable on this.

BR,
Jani.


>
> [1] https://patchwork.freedesktop.org/patch/68902/
> [2] https://patchwork.freedesktop.org/patch/69525/
>
>
>> 
>> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >  	else
>> >  		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
>> > @@ -476,8 +477,7 @@ static void ilk_audio_codec_enable(struct drm_connector
>> > *connector,
>> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> >  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> >  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
>> > -	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
>> > -	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
>> > +	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
>> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >  	else
>> >  		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
>> > -- 
>> > 2.4.3
>> > 
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

  reply	other threads:[~2016-03-14  6:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-11 15:33 [PATCH] drm/i915: Fix MST audio enabling Ander Conselvan de Oliveira
2016-03-11 15:37 ` Ville Syrjälä
2016-03-11 15:52   ` Ander Conselvan De Oliveira
2016-03-14  6:40     ` Jani Nikula [this message]
2016-03-11 17:03 ` ✗ Fi.CI.BAT: failure for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878u1l8suk.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=conselvan2@gmail.com \
    --cc=drm-intel-fixes@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=libin.yang@linux.intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.