On Thu, Apr 07, 2022 at 08:23:03PM +0300, Jani Nikula wrote:
> On Tue, 05 Apr 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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?
Yep. I'll try to clarify the commit msg a bit.
>
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > 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.
I can split that out for clarity. I didn't have these originally but
given the current reliance on the i915->vbt singleton I got a bit
scared what would happen on some weird machines with multiple panels.
IIRC some kind of native LVDS + SDVO LVDS machines may have existed
at some point.
Plenty of refactoring left here to split the parsed data to proper
per-panel things...
>
> Regardless,
>
> Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>
>
> > /* 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
--
Ville Syrjälä
Intel
