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