On Mon, 2020-08-24 at 16:57 -0700, Matt Roper wrote:
> On Mon, Aug 24, 2020 at 04:45:31PM -0700, Souza, Jose wrote:
> > On Mon, 2020-08-24 at 16:22 -0700, Matt Roper wrote:
> > > On Wed, Aug 19, 2020 at 11:51:44AM -0700, José Roberto de Souza wrote:
> > > > Reusing icl_get_combo_buf_trans() for eDP was causing the wrong table
> > > > being used when the eDP port don't support low power voltage swing
> > > > table.
> > > >
> > > > Cc: Lee Shawn C <
> > > > [email protected]
> > > >
> > > >
> > > > Cc: Khaled Almahallawy <
> > > > [email protected]
> > > >
> > > >
> > > > Signed-off-by: José Roberto de Souza <
> > > > [email protected]
> > > >
> > > >
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_ddi.c | 52 +++++++++++++++---------
> > > > 1 file changed, 33 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > index de5b216561d8..9a035bb7bd06 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > @@ -1088,30 +1088,44 @@ tgl_get_combo_buf_trans(struct intel_encoder
> > > > *encoder, int type, int rate,
> > > > {
> > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > >
> > > > - if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.hobl) {
> > > > - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > > -
> > > > - if (!intel_dp->hobl_failed && rate <= 540000) {
> > > > - /* Same table applies to TGL, RKL and DG1 */
> > > > - *n_entries =
> > > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > > > - return
> > > > tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > > > + switch (type) {
> > > > + case INTEL_OUTPUT_HDMI:
> > > > + *n_entries =
> > > > ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > > > + return icl_combo_phy_ddi_translations_hdmi;
> > > > + case INTEL_OUTPUT_EDP:
> > > > + if (dev_priv->vbt.edp.hobl) {
> > > > + struct intel_dp *intel_dp =
> > > > enc_to_intel_dp(encoder);
> > > > +
> > > > + if (!intel_dp->hobl_failed && rate <= 540000) {
> > > > + /* Same table applies to TGL, RKL and
> > > > DG1 */
> > > > + *n_entries =
> > > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > > > + return
> > > > tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > > > + }
> > > > }
> > > > - }
> > > >
> > > > - if (type == INTEL_OUTPUT_HDMI || type == INTEL_OUTPUT_EDP) {
> > > > - return icl_get_combo_buf_trans(encoder, type, rate,
> > > > n_entries);
> > > > - } else if (rate > 270000) {
> > > > - if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > > > - *n_entries =
> > > > ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> > > > - return
> > > > tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> > > > + if (rate > 540000) {
> > > > + *n_entries =
> > > > ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> > > > + return icl_combo_phy_ddi_translations_edp_hbr3;
> > >
> > > So if we have (HBR3 && !low_vswing) we still want to use the eDP table
> > > values? How did you figure that out? The only relevant comment I see
> > > in the bspec is
> > >
> > > eDP panels may support lower power, low voltage, swing values
> > > using the "eDP" protocol values from the table or higher power,
> > > high voltage, swing values using the "DP" protocol values.
> > >
> > > which doesn't make any specific mention of HBR3 being a special case.
> >
> > As combo ports in DP mode don't support HBR3 it is trying to use HBR3
>
> In that case should we drop the HBR3 check from the EHL patch? Because
> on EHL the DP combo PHYs actually can support HBR3.
Oh good catch will fix patch 3.
>
>
> Matt
>
> > table without even check if supported, in this case if the link
> > training fails intel_dp_get_link_train_fallback_values() will reduce
> > the rate and lanes until link training succeed or completely fails.
> >
> > But looks unlikely that a vendor will be ship a product with a HBR3
> > eDP panel and not set low_vswing in VBT.
> >
> > >
> > > Matt
> > >
> > > > + } else if (dev_priv->vbt.edp.low_vswing) {
> > > > + *n_entries =
> > > > ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr2);
> > > > + return icl_combo_phy_ddi_translations_edp_hbr2;
> > > > + }
> > > > + /* fall through */
> > > > + default:
> > > > + /* All combo DP and eDP ports that do not support
> > > > low_vswing */
> > > > + if (rate > 270000) {
> > > > + if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > > > + *n_entries =
> > > > ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> > > > + return
> > > > tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> > > > + }
> > > > +
> > > > + *n_entries =
> > > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> > > > + return tgl_combo_phy_ddi_translations_dp_hbr2;
> > > > }
> > > >
> > > > - *n_entries =
> > > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> > > > - return tgl_combo_phy_ddi_translations_dp_hbr2;
> > > > + *n_entries =
> > > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr);
> > > > + return tgl_combo_phy_ddi_translations_dp_hbr;
> > > > }
> > > > -
> > > > - *n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr);
> > > > - return tgl_combo_phy_ddi_translations_dp_hbr;
> > > > }
> > > >
> > > > static const struct tgl_dkl_phy_ddi_buf_trans *
> > > > --
> > > > 2.28.0
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > [email protected]
> > > >
> > > >
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > >
> > > >
> > >
> > >
>
>
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
