[Intel-gfx] [PATCH 5/9] drm/i915/display: move icl to description-based ddi init
Lucas De Marchi
lucas.de.marchi at gmail.com
Thu Jan 2 07:32:26 UTC 2020
On Tue, Dec 31, 2019 at 2:14 AM Jani Nikula <jani.nikula at linux.intel.com> wrote:
>
> On Mon, 23 Dec 2019, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
> > By adding a hook that determines if a port is present, we are able to
> > support Ice Lake in the new description-based DDI initialization.
> >
> > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 61 ++++++++++++++------
> > 1 file changed, 42 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index b3fb1e03cb0b..6b4d320ff92c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -16224,9 +16224,28 @@ static void intel_pps_init(struct drm_i915_private *dev_priv)
> > struct intel_output {
> > /* Initialize DSI if present */
> > void (*dsi_init)(struct drm_i915_private *i915);
> > +
> > + /*
> > + * Check if port is present before trying to initialize; if not provided
> > + * it's assumed the port is present (or we can't check and fail
> > + * gracefully
> > + */
> > + bool (*is_port_present)(struct drm_i915_private *i915,
> > + const struct intel_ddi_port_info *port_info);
> > +
> > struct intel_ddi_port_info ddi_ports[];
> > };
> >
> > +static bool icl_is_port_present(struct drm_i915_private *i915,
> > + const struct intel_ddi_port_info *port_info)
> > +{
> > + if (port_info->port != PORT_F)
> > + return true;
> > +
> > + return IS_ICL_WITH_PORT_F(i915) &&
> > + intel_bios_is_port_present(i915, PORT_F);
> > +}
> > +
>
> You know, all of that is here because there were some boards with broken
> VBTs claiming there was a port F on hardware that didn't have port
> F. And now we're turning it into infrastructure for all platforms. :(
>
> I actually preferred it when it was a localized hack for ICL. (Though I
> said at the time we should not add hacks for VBTs because this shit
> won't get fixed if we keep accommodating it.)
>
> If we still need the port F hack, I think I'd rather move it to
> intel_bios.c and skip port F description in VBT for platformsm that
> don't have it. So we can rely on VBT info elsewhere.
>
> Note that intel_ddi_init() will still check for VBT.
I don't think that is sufficient.... It may be a VBT thing, it may be a strap,
it may be because the phy is not hooked up (hence why we don't init
DDIC on TGL).
Idea here is to have a generic "is_port_present()" in which we collect
the N reasons why
we may not want to initialize a DDI.
The check intel_ddi_init() does is not sufficient for TGL, because the
VBT still says the port
is there, the port is in fact there, but the registers related to the
combophy don't behave.
Lucas De Marchi
>
> > static const struct intel_output tgl_output = {
> > .dsi_init = icl_dsi_init,
> > .ddi_ports = {
> > @@ -16242,6 +16261,20 @@ static const struct intel_output tgl_output = {
> > }
> > };
> >
> > +static const struct intel_output icl_output = {
> > + .dsi_init = icl_dsi_init,
> > + .is_port_present = icl_is_port_present,
> > + .ddi_ports = {
> > + { .port = PORT_A },
> > + { .port = PORT_B },
> > + { .port = PORT_C },
> > + { .port = PORT_D },
> > + { .port = PORT_E },
> > + { .port = PORT_F },
> > + { .port = PORT_NONE }
>
> At this stage of the series it seems to me we could have a ports mask in
> intel_device_info, and just loop over it using for_each_port_masked().
>
> BR,
> Jani.
>
> > + }
> > +};
> > +
> > static const struct intel_output ehl_output = {
> > .dsi_init = icl_dsi_init,
> > .ddi_ports = {
> > @@ -16276,12 +16309,19 @@ static void setup_ddi_outputs_desc(struct drm_i915_private *i915)
> > output = &tgl_output;
> > else if (IS_ELKHARTLAKE(i915))
> > output = &ehl_output;
> > + else if (IS_GEN(i915, 11))
> > + output = &icl_output;
> > else if (IS_GEN9_LP(i915))
> > output = &gen9lp_output;
> >
> > for (port_info = output->ddi_ports;
> > - port_info->port != PORT_NONE; port_info++)
> > + port_info->port != PORT_NONE; port_info++) {
> > + if (output->is_port_present &&
> > + !output->is_port_present(i915, port_info))
> > + continue;
> > +
> > intel_ddi_init(i915, port_info->port);
> > + }
> >
> > if (output->dsi_init)
> > output->dsi_init(i915);
> > @@ -16297,25 +16337,8 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
> > if (!HAS_DISPLAY(dev_priv) || !INTEL_DISPLAY_ENABLED(dev_priv))
> > return;
> >
> > - if (INTEL_GEN(dev_priv) >= 12 || IS_ELKHARTLAKE(dev_priv) ||
> > - IS_GEN9_LP(dev_priv)) {
> > + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> > setup_ddi_outputs_desc(dev_priv);
> > - } else if (IS_GEN(dev_priv, 11)) {
> > - intel_ddi_init(dev_priv, PORT_A);
> > - intel_ddi_init(dev_priv, PORT_B);
> > - intel_ddi_init(dev_priv, PORT_C);
> > - intel_ddi_init(dev_priv, PORT_D);
> > - intel_ddi_init(dev_priv, PORT_E);
> > - /*
> > - * On some ICL SKUs port F is not present. No strap bits for
> > - * this, so rely on VBT.
> > - * Work around broken VBTs on SKUs known to have no port F.
> > - */
> > - if (IS_ICL_WITH_PORT_F(dev_priv) &&
> > - intel_bios_is_port_present(dev_priv, PORT_F))
> > - intel_ddi_init(dev_priv, PORT_F);
> > -
> > - icl_dsi_init(dev_priv);
> > } else if (HAS_DDI(dev_priv)) {
> > int found;
>
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
More information about the Intel-gfx
mailing list