On Thu, Mar 31, 2022 at 04:04:57PM +0300, Jani Nikula wrote:
> On Thu, 31 Mar 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Rather than having the connector init get the fixed mode back from
> > intel_panel and then feed it straight back into intel_panel_init()
> > let's just make the fixed mode lookup put the mode directly onto
> > the panel's fixed_modes list. Avoids the pointless round trip and
> > opens the door for further enhancements to the fixed mode handling.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/icl_dsi.c | 8 +--
> > drivers/gpu/drm/i915/display/intel_dp.c | 22 ++++----
> > drivers/gpu/drm/i915/display/intel_drrs.c | 20 ++-----
> > drivers/gpu/drm/i915/display/intel_drrs.h | 3 +-
> > drivers/gpu/drm/i915/display/intel_dvo.c | 9 ++-
> > drivers/gpu/drm/i915/display/intel_lvds.c | 31 +++++------
> > drivers/gpu/drm/i915/display/intel_panel.c | 64 +++++++++-------------
> > drivers/gpu/drm/i915/display/intel_panel.h | 22 +++-----
> > drivers/gpu/drm/i915/display/intel_sdvo.c | 10 ++--
> > drivers/gpu/drm/i915/display/vlv_dsi.c | 9 +--
> > 10 files changed, 81 insertions(+), 117 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 44f4c65522b9..084cc51d1c41 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -1993,7 +1993,6 @@ void icl_dsi_init(struct drm_i915_private *dev_priv)
> > struct intel_encoder *encoder;
> > struct intel_connector *intel_connector;
> > struct drm_connector *connector;
> > - struct drm_display_mode *fixed_mode;
> > enum port port;
> >
> > if (!intel_bios_is_dsi_present(dev_priv, &port))
> > @@ -2050,15 +2049,16 @@ void icl_dsi_init(struct drm_i915_private *dev_priv)
> > intel_connector_attach_encoder(intel_connector, encoder);
> >
> > mutex_lock(&dev->mode_config.mutex);
> > - fixed_mode = intel_panel_vbt_lfp_fixed_mode(intel_connector);
> > + intel_panel_add_vbt_lfp_fixed_mode(intel_connector);
> > mutex_unlock(&dev->mode_config.mutex);
> >
> > - if (!fixed_mode) {
> > + if (!intel_panel_preferred_fixed_mode(intel_connector)) {
> > drm_err(&dev_priv->drm, "DSI fixed mode info missing\n");
> > goto err;
> > }
> >
> > - intel_panel_init(intel_connector, fixed_mode, NULL);
> > + intel_panel_init(intel_connector);
> > +
> > intel_backlight_setup(intel_connector, INVALID_PIPE);
> >
> > if (dev_priv->vbt.dsi.config->dual_link)
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 2c23ec0a880a..67472555211d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5015,8 +5015,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > struct drm_device *dev = &dev_priv->drm;
> > struct drm_connector *connector = &intel_connector->base;
> > - struct drm_display_mode *fixed_mode = NULL;
> > - struct drm_display_mode *downclock_mode = NULL;
> > + struct drm_display_mode *fixed_mode;
> > bool has_dpcd;
> > enum pipe pipe = INVALID_PIPE;
> > struct edid *edid;
> > @@ -5073,20 +5072,22 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > }
> > intel_connector->edid = edid;
> >
> > - fixed_mode = intel_panel_edid_fixed_mode(intel_connector);
> > - if (fixed_mode && intel_edp_has_drrs(intel_dp))
> > - downclock_mode = intel_drrs_init(intel_connector, fixed_mode);
> > + intel_panel_add_edid_fixed_mode(intel_connector);
> > + if (intel_panel_preferred_fixed_mode(intel_connector) &&
> > + intel_edp_has_drrs(intel_dp))
> > + intel_drrs_init(intel_connector);
> >
> > /* MSO requires information from the EDID */
> > intel_edp_mso_init(intel_dp);
> >
> > /* multiply the mode clock and horizontal timings for MSO */
> > - intel_edp_mso_mode_fixup(intel_connector, fixed_mode);
> > - intel_edp_mso_mode_fixup(intel_connector, downclock_mode);
> > + list_for_each_entry(fixed_mode, &intel_connector->panel.fixed_modes, head)
> > + intel_edp_mso_mode_fixup(intel_connector, fixed_mode);
> >
> > /* fallback to VBT if available for eDP */
> > - if (!fixed_mode)
> > - fixed_mode = intel_panel_vbt_lfp_fixed_mode(intel_connector);
> > + if (!intel_panel_preferred_fixed_mode(intel_connector))
> > + intel_panel_add_vbt_lfp_fixed_mode(intel_connector);
> > +
> > mutex_unlock(&dev->mode_config.mutex);
> >
> > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > @@ -5108,7 +5109,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > pipe_name(pipe));
> > }
> >
> > - intel_panel_init(intel_connector, fixed_mode, downclock_mode);
> > + intel_panel_init(intel_connector);
> > +
> > if (!(dev_priv->quirks & QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK))
> > intel_connector->panel.backlight.power = intel_pps_backlight_power;
> > intel_backlight_setup(intel_connector, pipe);
> > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c
> > index 1448c3029b8e..8fd280c7c83f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_drrs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c
> > @@ -358,7 +358,6 @@ void intel_crtc_drrs_init(struct intel_crtc *crtc)
> > /**
> > * intel_drrs_init - Init DRRS for eDP connector
> > * @connector: eDP connector
> > - * @fixed_mode: preferred mode of panel
> > *
> > * This function is called only once at driver load to initialize
> > * DRRS support for the connector.
> > @@ -368,25 +367,14 @@ void intel_crtc_drrs_init(struct intel_crtc *crtc)
> > * DRRS support is determined by the presence of downclock mode (apart
> > * from VBT setting).
> > */
> > -struct drm_display_mode *
> > -intel_drrs_init(struct intel_connector *connector,
> > - const struct drm_display_mode *fixed_mode)
> > +void intel_drrs_init(struct intel_connector *connector)
> > {
> > struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > - struct drm_display_mode *downclock_mode;
> > -
> > - downclock_mode = intel_panel_edid_downclock_mode(connector, fixed_mode);
> > - if (!downclock_mode) {
> > - drm_dbg_kms(&dev_priv->drm,
> > - "[CONNECTOR:%d:%s] DRRS not supported due to lack of downclock mode\n",
> > - connector->base.base.id, connector->base.name);
> > - return NULL;
> > - }
> > +
> > + intel_panel_add_edid_downclock_mode(connector);
>
> What if there's no downclock mode, just one native mode with one
> vrefresh? We'll now now say drrs type is this or that below, but it'll
> never get enabled in compute config?
I meant to use intel_panel_drrs_type() here. But looks like I
fumbled that a bit and only did it in patch 4.
>
> Should intel_panel_add_edid_downclock_mode() debug log when there's no
> downclock mode?
Hmm. Maybe. currently we only log the modes we're going to use, not the
ones we discard.
--
Ville Syrjälä
Intel
