Re: [PATCH 4.8 118/138] drm/i915: Clean up DDI DDC/AUX CH sanitation

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Nov 10, 2016 at 09:16:49PM +0100, Maarten Maathuis wrote:
> Hi,
> 
> I'm merely curious why this patch isn't also included:
> https://cgit.freedesktop.org/drm-intel/commit/?id=e4ab73a13291fc844c9e24d5c347bd95818544d2
> 
> When i checked it, it's also not in 4.9-rc git tree.
> This patch affects HDMI, and the HDMI connector was "my problem".
> 
> @Ville: Is it intentional this one was left out?

Hmm. There was cc:stable+fixes:+bugzilla link on that one as well.
Not sure why it got left behind. Jani, any ideas what happened there?

> 
> Maarten.
> 
> On Wed, Nov 9, 2016 at 11:46 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > 4.8-stable review patch.  If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> >
> > commit 0ce140d45a8398b501934ac289aef0eb7f47c596 upstream.
> >
> > Now that we use the AUX and GMBUS assignment from VBT for all ports,
> > let's clean up the sanitization of the port information a bit.
> > Previosuly we only did this for port E, and only complained about a
> > non-standard assignment for the other ports. But as we know that
> > non-standard assignments are a fact of life, let's expand the
> > sanitization to all the ports.
> >
> > v2: Include a commit message, fix up the comments a bit
> > v3: Don't clobber other ports if the current port has no alternate aux ch/ddc pin
> >
> > Cc: Maarten Maathuis <madman2003@gmail.com>
> > Tested-by: Maarten Maathuis <madman2003@gmail.com>
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > Link: http://patchwork.freedesktop.org/patch/msgid/1476208368-5710-4-git-send-email-ville.syrjala@linux.intel.com
> > Reviewed-by: Jim Bride <jim.bride@linux.intel.com> (v2)
> > (cherry picked from commit 9454fa871edf15c20a0371548b3ec0d6d944a498)
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > ---
> >  drivers/gpu/drm/i915/intel_bios.c |  122 +++++++++++++++++++++++---------------
> >  1 file changed, 77 insertions(+), 45 deletions(-)
> >
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -1031,6 +1031,77 @@ static u8 translate_iboost(u8 val)
> >         return mapping[val];
> >  }
> >
> > +static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
> > +                            enum port port)
> > +{
> > +       const struct ddi_vbt_port_info *info =
> > +               &dev_priv->vbt.ddi_port_info[port];
> > +       enum port p;
> > +
> > +       if (!info->alternate_ddc_pin)
> > +               return;
> > +
> > +       for_each_port_masked(p, (1 << port) - 1) {
> > +               struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> > +
> > +               if (info->alternate_ddc_pin != i->alternate_ddc_pin)
> > +                       continue;
> > +
> > +               DRM_DEBUG_KMS("port %c trying to use the same DDC pin (0x%x) as port %c, "
> > +                             "disabling port %c DVI/HDMI support\n",
> > +                             port_name(p), i->alternate_ddc_pin,
> > +                             port_name(port), port_name(p));
> > +
> > +               /*
> > +                * If we have multiple ports supposedly sharing the
> > +                * pin, then dvi/hdmi couldn't exist on the shared
> > +                * port. Otherwise they share the same ddc bin and
> > +                * system couldn't communicate with them separately.
> > +                *
> > +                * Due to parsing the ports in alphabetical order,
> > +                * a higher port will always clobber a lower one.
> > +                */
> > +               i->supports_dvi = false;
> > +               i->supports_hdmi = false;
> > +               i->alternate_ddc_pin = 0;
> > +       }
> > +}
> > +
> > +static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
> > +                           enum port port)
> > +{
> > +       const struct ddi_vbt_port_info *info =
> > +               &dev_priv->vbt.ddi_port_info[port];
> > +       enum port p;
> > +
> > +       if (!info->alternate_aux_channel)
> > +               return;
> > +
> > +       for_each_port_masked(p, (1 << port) - 1) {
> > +               struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> > +
> > +               if (info->alternate_aux_channel != i->alternate_aux_channel)
> > +                       continue;
> > +
> > +               DRM_DEBUG_KMS("port %c trying to use the same AUX CH (0x%x) as port %c, "
> > +                             "disabling port %c DP support\n",
> > +                             port_name(p), i->alternate_aux_channel,
> > +                             port_name(port), port_name(p));
> > +
> > +               /*
> > +                * If we have multiple ports supposedlt sharing the
> > +                * aux channel, then DP couldn't exist on the shared
> > +                * port. Otherwise they share the same aux channel
> > +                * and system couldn't communicate with them separately.
> > +                *
> > +                * Due to parsing the ports in alphabetical order,
> > +                * a higher port will always clobber a lower one.
> > +                */
> > +               i->supports_dp = false;
> > +               i->alternate_aux_channel = 0;
> > +       }
> > +}
> > +
> >  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> >                            const struct bdb_header *bdb)
> >  {
> > @@ -1105,54 +1176,15 @@ static void parse_ddi_port(struct drm_i9
> >                 DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
> >
> >         if (is_dvi) {
> > -               if (port == PORT_E) {
> > -                       info->alternate_ddc_pin = ddc_pin;
> > -                       /* if DDIE share ddc pin with other port, then
> > -                        * dvi/hdmi couldn't exist on the shared port.
> > -                        * Otherwise they share the same ddc bin and system
> > -                        * couldn't communicate with them seperately. */
> > -                       if (ddc_pin == DDC_PIN_B) {
> > -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
> > -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
> > -                       } else if (ddc_pin == DDC_PIN_C) {
> > -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
> > -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
> > -                       } else if (ddc_pin == DDC_PIN_D) {
> > -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
> > -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
> > -                       }
> > -               } else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> > -                       DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> > -               else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> > -                       DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> > -               else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> > -                       DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> > +               info->alternate_ddc_pin = ddc_pin;
> > +
> > +               sanitize_ddc_pin(dev_priv, port);
> >         }
> >
> >         if (is_dp) {
> > -               if (port == PORT_E) {
> > -                       info->alternate_aux_channel = aux_channel;
> > -                       /* if DDIE share aux channel with other port, then
> > -                        * DP couldn't exist on the shared port. Otherwise
> > -                        * they share the same aux channel and system
> > -                        * couldn't communicate with them seperately. */
> > -                       if (aux_channel == DP_AUX_A)
> > -                               dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0;
> > -                       else if (aux_channel == DP_AUX_B)
> > -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0;
> > -                       else if (aux_channel == DP_AUX_C)
> > -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0;
> > -                       else if (aux_channel == DP_AUX_D)
> > -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0;
> > -               }
> > -               else if (aux_channel == DP_AUX_A && port != PORT_A)
> > -                       DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
> > -               else if (aux_channel == DP_AUX_B && port != PORT_B)
> > -                       DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
> > -               else if (aux_channel == DP_AUX_C && port != PORT_C)
> > -                       DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
> > -               else if (aux_channel == DP_AUX_D && port != PORT_D)
> > -                       DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
> > +               info->alternate_aux_channel = aux_channel;
> > +
> > +               sanitize_aux_ch(dev_priv, port);
> >         }
> >
> >         if (bdb->version >= 158) {
> >
> >
> 
> 
> 
> -- 
> Far away from the primal instinct, the song seems to fade away, the
> river get wider between your thoughts and the things we do and say.

-- 
Ville Syrj�l�
Intel OTC