Re: [Intel-gfx] [PATCH v2 03/11] drm/i915: Refactor non-EDID fixed mode duplication

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 31 Mar 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> All the non-EDID fixed mode functions basically do the exact
> same thing. Let's refactor the common bits into a shared
> function.
>
> There are minor differences on how the mode types are populated,
> whether the display info physical size is updated, and the debug
> print. The differences are purely accidental, so unifying them is
> actually a good thing.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_panel.c | 66 ++++++++++------------
>  1 file changed, 30 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index 415aa381f732..1e56ca9033e0 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -243,68 +243,62 @@ void intel_panel_add_edid_fixed_mode(struct intel_connector *connector)
>  	list_add_tail(&fixed_mode->head, &connector->panel.fixed_modes);
>  }
>  
> -void intel_panel_add_vbt_lfp_fixed_mode(struct intel_connector *connector)
> +static void intel_panel_add_fixed_mode(struct intel_connector *connector,
> +				       struct drm_display_mode *fixed_mode,
> +				       const char *type)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +	struct drm_i915_private *i915 = to_i915(connector->base.dev);
>  	struct drm_display_info *info = &connector->base.display_info;
> -	struct drm_display_mode *fixed_mode;
>  
> -	if (!dev_priv->vbt.lfp_lvds_vbt_mode)
> -		return;
> -
> -	fixed_mode = drm_mode_duplicate(&dev_priv->drm,
> -					dev_priv->vbt.lfp_lvds_vbt_mode);
>  	if (!fixed_mode)
>  		return;
>  
> -	fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> -
> -	drm_dbg_kms(&dev_priv->drm, "[CONNECTOR:%d:%s] using mode from VBT: " DRM_MODE_FMT "\n",
> -		    connector->base.base.id, connector->base.name,
> -		    DRM_MODE_ARG(fixed_mode));
> +	fixed_mode->type |= DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
>  
>  	info->width_mm = fixed_mode->width_mm;
>  	info->height_mm = fixed_mode->height_mm;
>  
> +	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] using %s fixed mode: " DRM_MODE_FMT "\n",
> +		    connector->base.base.id, connector->base.name, type,
> +		    DRM_MODE_ARG(fixed_mode));
> +
>  	list_add_tail(&fixed_mode->head, &connector->panel.fixed_modes);
>  }
>  
> -void intel_panel_add_vbt_sdvo_fixed_mode(struct intel_connector *connector)
> +void intel_panel_add_vbt_lfp_fixed_mode(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
> -	struct drm_display_mode *fixed_mode;
> -
> -	if (!i915->vbt.sdvo_lvds_vbt_mode)
> -		return;
> +	const struct drm_display_mode *mode;
>  
> -	fixed_mode = drm_mode_duplicate(&i915->drm,
> -					i915->vbt.sdvo_lvds_vbt_mode);
> -	if (!fixed_mode)
> +	mode = i915->vbt.lfp_lvds_vbt_mode;
> +	if (!mode)
>  		return;
>  
> -	/* Guarantee the mode is preferred */
> -	fixed_mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
> -
> -	list_add_tail(&fixed_mode->head, &connector->panel.fixed_modes);
> +	intel_panel_add_fixed_mode(connector,
> +				   drm_mode_duplicate(&i915->drm, mode),
> +				   "VBT LFP");
>  }
>  
> -void intel_panel_add_encoder_fixed_mode(struct intel_connector *connector,
> -					struct intel_encoder *encoder)
> +void intel_panel_add_vbt_sdvo_fixed_mode(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
> -	struct drm_display_mode *fixed_mode;
> +	const struct drm_display_mode *mode;
>  
> -	fixed_mode = intel_encoder_current_mode(encoder);
> -	if (!fixed_mode)
> +	mode = i915->vbt.sdvo_lvds_vbt_mode;
> +	if (!mode)
>  		return;
>  
> -	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] using current (BIOS) mode: " DRM_MODE_FMT "\n",
> -		    connector->base.base.id, connector->base.name,
> -		    DRM_MODE_ARG(fixed_mode));
> -
> -	fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> +	intel_panel_add_fixed_mode(connector,
> +				   drm_mode_duplicate(&i915->drm, mode),
> +				   "VBT SDVO");
> +}
>  
> -	list_add_tail(&fixed_mode->head, &connector->panel.fixed_modes);
> +void intel_panel_add_encoder_fixed_mode(struct intel_connector *connector,
> +					struct intel_encoder *encoder)
> +{
> +	intel_panel_add_fixed_mode(connector,
> +				   intel_encoder_current_mode(encoder),
> +				   "current (BIOS)");
>  }
>  
>  /* adjusted_mode has been preset to be the panel's fixed mode */

-- 
Jani Nikula, Intel Open Source Graphics Center