Re: [Intel-gfx] [PATCH v2 12/22] drm/i915/bios: Split VBT parsing to global vs. panel specific parts

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

On Tue, 05 Apr 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Parsing the panel specific data from VBT is currently happening
> too early. Split the whole thing into global vs. panel specific
> parts so that we can start doing the panel specific parsing at
> a later time.

Might want to mention panel_type here somewhere, that's basically the
split, right?

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c    | 50 +++++++++++++-------
>  drivers/gpu/drm/i915/display/intel_bios.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_display.c |  1 +
>  3 files changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 1a7f1aa79827..da2b1932e10d 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -723,6 +723,9 @@ parse_generic_dtd(struct drm_i915_private *i915)
>  	struct drm_display_mode *panel_fixed_mode;
>  	int num_dtd;
>  
> +	if (i915->vbt.lfp_lvds_vbt_mode)
> +		return;
> +
>  	generic_dtd = find_section(i915, BDB_GENERIC_DTD);
>  	if (!generic_dtd)
>  		return;
> @@ -891,6 +894,9 @@ parse_sdvo_panel_data(struct drm_i915_private *i915)
>  	struct drm_display_mode *panel_fixed_mode;
>  	int index;
>  
> +	if (i915->vbt.sdvo_lvds_vbt_mode)
> +		return;
> +
>  	index = i915->params.vbt_sdvo_panel_type;
>  	if (index == -2) {
>  		drm_dbg_kms(&i915->drm,
> @@ -1419,6 +1425,9 @@ parse_mipi_config(struct drm_i915_private *i915)
>  	int panel_type = i915->vbt.panel_type;
>  	enum port port;
>  
> +	if (i915->vbt.dsi.config)
> +		return;
> +
>  	/* parse MIPI blocks only if LFP type is MIPI */
>  	if (!intel_bios_is_dsi_present(i915, &port))
>  		return;
> @@ -1739,6 +1748,9 @@ parse_mipi_sequence(struct drm_i915_private *i915)
>  	u8 *data;
>  	int index = 0;
>  
> +	if (i915->vbt.dsi.data)
> +		return;
> +

All of the above checks to (presumably) allow calling
intel_bios_init_panel() multiple times feel a bit out of place here. At
the very least need a mention in the commit message.

Regardless,

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


>  	/* Only our generic panel driver uses the sequence block. */
>  	if (i915->vbt.dsi.panel_id != MIPI_DSI_GENERIC_PANEL_ID)
>  		return;
> @@ -2878,6 +2890,27 @@ void intel_bios_init(struct drm_i915_private *i915)
>  	/* Grab useful general definitions */
>  	parse_general_features(i915);
>  	parse_general_definitions(i915);
> +	parse_driver_features(i915);
> +
> +	/* Depends on child device list */
> +	parse_compression_parameters(i915);
> +
> +out:
> +	if (!vbt) {
> +		drm_info(&i915->drm,
> +			 "Failed to find VBIOS tables (VBT)\n");
> +		init_vbt_missing_defaults(i915);
> +	}
> +
> +	/* Further processing on pre-parsed or generated child device data */
> +	parse_sdvo_device_mapping(i915);
> +	parse_ddi_ports(i915);
> +
> +	kfree(oprom_vbt);
> +}
> +
> +void intel_bios_init_panel(struct drm_i915_private *i915)
> +{
>  	parse_panel_options(i915);
>  	/*
>  	 * Older VBTs provided DTD information for internal displays through
> @@ -2892,29 +2925,12 @@ void intel_bios_init(struct drm_i915_private *i915)
>  	parse_lfp_data(i915);
>  	parse_lfp_backlight(i915);
>  	parse_sdvo_panel_data(i915);
> -	parse_driver_features(i915);
>  	parse_panel_driver_features(i915);
>  	parse_power_conservation_features(i915);
>  	parse_edp(i915);
>  	parse_psr(i915);
>  	parse_mipi_config(i915);
>  	parse_mipi_sequence(i915);
> -
> -	/* Depends on child device list */
> -	parse_compression_parameters(i915);
> -
> -out:
> -	if (!vbt) {
> -		drm_info(&i915->drm,
> -			 "Failed to find VBIOS tables (VBT)\n");
> -		init_vbt_missing_defaults(i915);
> -	}
> -
> -	/* Further processing on pre-parsed or generated child device data */
> -	parse_sdvo_device_mapping(i915);
> -	parse_ddi_ports(i915);
> -
> -	kfree(oprom_vbt);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
> index 4709c4d29805..c744d75fa435 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> @@ -230,6 +230,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);
>  void intel_bios_driver_remove(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>  bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index eee185ed41c3..4ece4e7d0296 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9650,6 +9650,7 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	}
>  
>  	intel_bios_init(i915);
> +	intel_bios_init_panel(i915);
>  
>  	ret = intel_vga_register(i915);
>  	if (ret)

-- 
Jani Nikula, Intel Open Source Graphics Center