| Message ID | 20220927174518.30230-2-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup | expand |
On Tue, 27 Sep 2022, Animesh Manna <animesh.manna@intel.com> wrote: > From display gen12 onwards to support dual EDP two instances of pps added. > Currently backlight controller and pps instance can be mapped together > for a specific panel. Extended support for gen12 for dual EDP usage. Frankly the dual PPS support was there already, but broken. This fixes it, and enables it for display 12+. > > v1: Iniital revision. > v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani] > v3: Set pps_id to -1 for pnpid type of panel which will be used by > bxt_power_sequencer_idx() to set 2nd pps instance as default for > 2nd EDP panel. [Jani] > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Uma Shankar <uma.shankar@intel.com> > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_bios.c | 13 +++++++++++-- > drivers/gpu/drm/i915/display/intel_bios.h | 2 +- > drivers/gpu/drm/i915/display/intel_display_types.h | 1 + > drivers/gpu/drm/i915/display/intel_dp.c | 10 +++++++--- > drivers/gpu/drm/i915/display/intel_pps.c | 12 +++++++++++- > 5 files changed, 31 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > index 28bdb936cd1f..2015b6592754 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -3175,13 +3175,14 @@ void intel_bios_init(struct drm_i915_private *i915) > kfree(oprom_vbt); > } > > -void intel_bios_init_panel(struct drm_i915_private *i915, > +bool intel_bios_init_panel(struct drm_i915_private *i915, > struct intel_panel *panel, > const struct intel_bios_encoder_data *devdata, > const struct edid *edid) > { > - init_vbt_panel_defaults(panel); > + bool retry = false; > > + init_vbt_panel_defaults(panel); > panel->vbt.panel_type = get_panel_type(i915, devdata, edid); > > parse_panel_options(i915, panel); > @@ -3195,6 +3196,14 @@ void intel_bios_init_panel(struct drm_i915_private *i915, > parse_psr(i915, panel); > parse_mipi_config(i915, panel); > parse_mipi_sequence(i915, panel); > + > + if (panel->vbt.panel_type == PANEL_TYPE_PNPID || > + panel->vbt.panel_type == PANEL_TYPE_FALLBACK) { > + panel->vbt.edp.pps_id = -1; > + retry = true; > + } Why do you initialize everything above if you know the panel type is garbage and we need to retry? I don't think the above functions were designed with the idea they could be called multiple times for the same panel. Return early if you know it's wrong. Also, don't return true if the EDID is provided. BR, Jani. > + > + return retry; > } > > /** > diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h > index e375405a7828..f8ef0274f3ee 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.h > +++ b/drivers/gpu/drm/i915/display/intel_bios.h > @@ -232,7 +232,7 @@ struct mipi_pps_data { > } __packed; > > void intel_bios_init(struct drm_i915_private *dev_priv); > -void intel_bios_init_panel(struct drm_i915_private *dev_priv, > +bool intel_bios_init_panel(struct drm_i915_private *dev_priv, > struct intel_panel *panel, > const struct intel_bios_encoder_data *devdata, > const struct edid *edid); > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index b78b29951241..0edc0b8f3743 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -307,6 +307,7 @@ struct intel_vbt_panel_data { > int preemphasis; > int vswing; > int bpp; > + int pps_id; > struct edp_power_seq pps; > u8 drrs_msa_timing_delay; > bool low_vswing; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index c19e99ee06b6..a94fc947cdb3 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5203,6 +5203,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > bool has_dpcd; > enum pipe pipe = INVALID_PIPE; > struct edid *edid; > + bool retry; > > if (!intel_dp_is_edp(intel_dp)) > return true; > @@ -5222,6 +5223,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > return false; > } > > + retry = intel_bios_init_panel(dev_priv, &intel_connector->panel, > + encoder->devdata, NULL); > + > intel_pps_init(intel_dp); > > /* Cache DPCD and EDID for edp. */ > @@ -5255,9 +5259,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > edid = ERR_PTR(-ENOENT); > } > intel_connector->edid = edid; > - > - intel_bios_init_panel(dev_priv, &intel_connector->panel, > - encoder->devdata, IS_ERR(edid) ? NULL : edid); > + if (retry) > + intel_bios_init_panel(dev_priv, &intel_connector->panel, > + encoder->devdata, IS_ERR(edid) ? NULL : edid); > > intel_panel_add_edid_fixed_modes(intel_connector, > intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE, > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c > index b972fa6ec00d..da98b180639a 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.c > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > @@ -218,6 +218,16 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > /* We should never land here with regular DP ports */ > drm_WARN_ON(&dev_priv->drm, !intel_dp_is_edp(intel_dp)); > > + if (connector->panel.vbt.edp.pps_id == -1) { > + /* > + * Use 2nd PPS instance as default for 2nd EDP panel. > + */ > + if (connector->encoder->port == PORT_A) > + return 0; > + else > + return 1; > + } > + > if (!intel_dp->pps.pps_reset) > return backlight_controller; > > @@ -1430,7 +1440,7 @@ void intel_pps_init(struct intel_dp *intel_dp) > intel_dp->pps.initializing = true; > INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work); > > - if (IS_GEMINILAKE(i915) || IS_BROXTON(i915)) > + if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12) > intel_dp->get_pps_idx = bxt_power_sequencer_idx; > else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) > intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
> -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: Tuesday, October 4, 2022 1:19 PM > To: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Manna, Animesh <animesh.manna@intel.com>; Ville Syrjälä > <ville.syrjala@linux.intel.com>; Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [PATCH 2/2] drm/i915/pps: Enable 2nd pps for dual EDP scenario > > On Tue, 27 Sep 2022, Animesh Manna <animesh.manna@intel.com> wrote: > > From display gen12 onwards to support dual EDP two instances of pps added. > > Currently backlight controller and pps instance can be mapped together > > for a specific panel. Extended support for gen12 for dual EDP usage. > > Frankly the dual PPS support was there already, but broken. This fixes it, and > enables it for display 12+. > > > > > v1: Iniital revision. > > v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. > > [Jani] > > v3: Set pps_id to -1 for pnpid type of panel which will be used by > > bxt_power_sequencer_idx() to set 2nd pps instance as default for 2nd > > EDP panel. [Jani] > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Uma Shankar <uma.shankar@intel.com> > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_bios.c | 13 +++++++++++-- > > drivers/gpu/drm/i915/display/intel_bios.h | 2 +- > > drivers/gpu/drm/i915/display/intel_display_types.h | 1 + > > drivers/gpu/drm/i915/display/intel_dp.c | 10 +++++++--- > > drivers/gpu/drm/i915/display/intel_pps.c | 12 +++++++++++- > > 5 files changed, 31 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > > b/drivers/gpu/drm/i915/display/intel_bios.c > > index 28bdb936cd1f..2015b6592754 100644 > > --- a/drivers/gpu/drm/i915/display/intel_bios.c > > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > > @@ -3175,13 +3175,14 @@ void intel_bios_init(struct drm_i915_private > *i915) > > kfree(oprom_vbt); > > } > > > > -void intel_bios_init_panel(struct drm_i915_private *i915, > > +bool intel_bios_init_panel(struct drm_i915_private *i915, > > struct intel_panel *panel, > > const struct intel_bios_encoder_data *devdata, > > const struct edid *edid) > > { > > - init_vbt_panel_defaults(panel); > > + bool retry = false; > > > > + init_vbt_panel_defaults(panel); > > panel->vbt.panel_type = get_panel_type(i915, devdata, edid); > > > > parse_panel_options(i915, panel); > > @@ -3195,6 +3196,14 @@ void intel_bios_init_panel(struct drm_i915_private > *i915, > > parse_psr(i915, panel); > > parse_mipi_config(i915, panel); > > parse_mipi_sequence(i915, panel); > > + > > + if (panel->vbt.panel_type == PANEL_TYPE_PNPID || > > + panel->vbt.panel_type == PANEL_TYPE_FALLBACK) { > > + panel->vbt.edp.pps_id = -1; > > + retry = true; > > + } > > Why do you initialize everything above if you know the panel type is garbage and > we need to retry? I don't think the above functions were designed with the idea > they could be called multiple times for the same panel. > > Return early if you know it's wrong. Also, don't return true if the EDID is > provided. Thanks Jani for review, have sent the v4 after addressing the above comments. Regards, Animesh > > BR, > Jani. > > > + > > + return retry; > > } > > > > /** > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.h > > b/drivers/gpu/drm/i915/display/intel_bios.h > > index e375405a7828..f8ef0274f3ee 100644 > > --- a/drivers/gpu/drm/i915/display/intel_bios.h > > +++ b/drivers/gpu/drm/i915/display/intel_bios.h > > @@ -232,7 +232,7 @@ struct mipi_pps_data { } __packed; > > > > void intel_bios_init(struct drm_i915_private *dev_priv); -void > > intel_bios_init_panel(struct drm_i915_private *dev_priv, > > +bool intel_bios_init_panel(struct drm_i915_private *dev_priv, > > struct intel_panel *panel, > > const struct intel_bios_encoder_data *devdata, > > const struct edid *edid); > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index b78b29951241..0edc0b8f3743 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -307,6 +307,7 @@ struct intel_vbt_panel_data { > > int preemphasis; > > int vswing; > > int bpp; > > + int pps_id; > > struct edp_power_seq pps; > > u8 drrs_msa_timing_delay; > > bool low_vswing; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index c19e99ee06b6..a94fc947cdb3 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -5203,6 +5203,7 @@ static bool intel_edp_init_connector(struct intel_dp > *intel_dp, > > bool has_dpcd; > > enum pipe pipe = INVALID_PIPE; > > struct edid *edid; > > + bool retry; > > > > if (!intel_dp_is_edp(intel_dp)) > > return true; > > @@ -5222,6 +5223,9 @@ static bool intel_edp_init_connector(struct intel_dp > *intel_dp, > > return false; > > } > > > > + retry = intel_bios_init_panel(dev_priv, &intel_connector->panel, > > + encoder->devdata, NULL); > > + > > intel_pps_init(intel_dp); > > > > /* Cache DPCD and EDID for edp. */ > > @@ -5255,9 +5259,9 @@ static bool intel_edp_init_connector(struct intel_dp > *intel_dp, > > edid = ERR_PTR(-ENOENT); > > } > > intel_connector->edid = edid; > > - > > - intel_bios_init_panel(dev_priv, &intel_connector->panel, > > - encoder->devdata, IS_ERR(edid) ? NULL : edid); > > + if (retry) > > + intel_bios_init_panel(dev_priv, &intel_connector->panel, > > + encoder->devdata, IS_ERR(edid) ? NULL : > edid); > > > > intel_panel_add_edid_fixed_modes(intel_connector, > > intel_connector->panel.vbt.drrs_type > != DRRS_TYPE_NONE, diff > > --git a/drivers/gpu/drm/i915/display/intel_pps.c > > b/drivers/gpu/drm/i915/display/intel_pps.c > > index b972fa6ec00d..da98b180639a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_pps.c > > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > > @@ -218,6 +218,16 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > > /* We should never land here with regular DP ports */ > > drm_WARN_ON(&dev_priv->drm, !intel_dp_is_edp(intel_dp)); > > > > + if (connector->panel.vbt.edp.pps_id == -1) { > > + /* > > + * Use 2nd PPS instance as default for 2nd EDP panel. > > + */ > > + if (connector->encoder->port == PORT_A) > > + return 0; > > + else > > + return 1; > > + } > > + > > if (!intel_dp->pps.pps_reset) > > return backlight_controller; > > > > @@ -1430,7 +1440,7 @@ void intel_pps_init(struct intel_dp *intel_dp) > > intel_dp->pps.initializing = true; > > INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, > > edp_panel_vdd_work); > > > > - if (IS_GEMINILAKE(i915) || IS_BROXTON(i915)) > > + if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= > > +12) > > intel_dp->get_pps_idx = bxt_power_sequencer_idx; > > else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) > > intel_dp->get_pps_idx = vlv_power_sequencer_pipe; > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index 28bdb936cd1f..2015b6592754 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -3175,13 +3175,14 @@ void intel_bios_init(struct drm_i915_private *i915) kfree(oprom_vbt); } -void intel_bios_init_panel(struct drm_i915_private *i915, +bool intel_bios_init_panel(struct drm_i915_private *i915, struct intel_panel *panel, const struct intel_bios_encoder_data *devdata, const struct edid *edid) { - init_vbt_panel_defaults(panel); + bool retry = false; + init_vbt_panel_defaults(panel); panel->vbt.panel_type = get_panel_type(i915, devdata, edid); parse_panel_options(i915, panel); @@ -3195,6 +3196,14 @@ void intel_bios_init_panel(struct drm_i915_private *i915, parse_psr(i915, panel); parse_mipi_config(i915, panel); parse_mipi_sequence(i915, panel); + + if (panel->vbt.panel_type == PANEL_TYPE_PNPID || + panel->vbt.panel_type == PANEL_TYPE_FALLBACK) { + panel->vbt.edp.pps_id = -1; + retry = true; + } + + return retry; } /** diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h index e375405a7828..f8ef0274f3ee 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.h +++ b/drivers/gpu/drm/i915/display/intel_bios.h @@ -232,7 +232,7 @@ struct mipi_pps_data { } __packed; void intel_bios_init(struct drm_i915_private *dev_priv); -void intel_bios_init_panel(struct drm_i915_private *dev_priv, +bool intel_bios_init_panel(struct drm_i915_private *dev_priv, struct intel_panel *panel, const struct intel_bios_encoder_data *devdata, const struct edid *edid); diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index b78b29951241..0edc0b8f3743 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -307,6 +307,7 @@ struct intel_vbt_panel_data { int preemphasis; int vswing; int bpp; + int pps_id; struct edp_power_seq pps; u8 drrs_msa_timing_delay; bool low_vswing; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index c19e99ee06b6..a94fc947cdb3 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5203,6 +5203,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, bool has_dpcd; enum pipe pipe = INVALID_PIPE; struct edid *edid; + bool retry; if (!intel_dp_is_edp(intel_dp)) return true; @@ -5222,6 +5223,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, return false; } + retry = intel_bios_init_panel(dev_priv, &intel_connector->panel, + encoder->devdata, NULL); + intel_pps_init(intel_dp); /* Cache DPCD and EDID for edp. */ @@ -5255,9 +5259,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, edid = ERR_PTR(-ENOENT); } intel_connector->edid = edid; - - intel_bios_init_panel(dev_priv, &intel_connector->panel, - encoder->devdata, IS_ERR(edid) ? NULL : edid); + if (retry) + intel_bios_init_panel(dev_priv, &intel_connector->panel, + encoder->devdata, IS_ERR(edid) ? NULL : edid); intel_panel_add_edid_fixed_modes(intel_connector, intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE, diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c index b972fa6ec00d..da98b180639a 100644 --- a/drivers/gpu/drm/i915/display/intel_pps.c +++ b/drivers/gpu/drm/i915/display/intel_pps.c @@ -218,6 +218,16 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) /* We should never land here with regular DP ports */ drm_WARN_ON(&dev_priv->drm, !intel_dp_is_edp(intel_dp)); + if (connector->panel.vbt.edp.pps_id == -1) { + /* + * Use 2nd PPS instance as default for 2nd EDP panel. + */ + if (connector->encoder->port == PORT_A) + return 0; + else + return 1; + } + if (!intel_dp->pps.pps_reset) return backlight_controller; @@ -1430,7 +1440,7 @@ void intel_pps_init(struct intel_dp *intel_dp) intel_dp->pps.initializing = true; INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work); - if (IS_GEMINILAKE(i915) || IS_BROXTON(i915)) + if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12) intel_dp->get_pps_idx = bxt_power_sequencer_idx; else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
From display gen12 onwards to support dual EDP two instances of pps added. Currently backlight controller and pps instance can be mapped together for a specific panel. Extended support for gen12 for dual EDP usage. v1: Iniital revision. v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani] v3: Set pps_id to -1 for pnpid type of panel which will be used by bxt_power_sequencer_idx() to set 2nd pps instance as default for 2nd EDP panel. [Jani] Cc: Jani Nikula <jani.nikula@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Uma Shankar <uma.shankar@intel.com> Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_bios.c | 13 +++++++++++-- drivers/gpu/drm/i915/display/intel_bios.h | 2 +- drivers/gpu/drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/i915/display/intel_dp.c | 10 +++++++--- drivers/gpu/drm/i915/display/intel_pps.c | 12 +++++++++++- 5 files changed, 31 insertions(+), 7 deletions(-)