From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/6] drm/i915: Do panel VBT init early if the VBT declares an explicit panel type
Date: Wed, 09 Nov 2022 16:59:10 +0200 [thread overview]
Message-ID: <875yfo6uo1.fsf@intel.com> (raw)
In-Reply-To: <20221109111649.23062-3-ville.syrjala@linux.intel.com>
On Wed, 09 Nov 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Lots of ADL machines out there with bogus VBTs that declare
> two eDP child devices. In order for those to work we need to
> figure out which power sequencer to use before we try the EDID
> read. So let's do the panel VBT init early if we can, falling
> back to the post-EDID init otherwise.
>
> The post-EDID init panel_type=0xff approach of assuming the
> power sequencer should already be enabled doesn't really work
> with multiple eDP panels, and currently we just end up using
> the same power sequencer for both eDP ports, which at least
> confuses the wakeref tracking, and potentially also causes us
> to toggle the VDD for the panel when we should not.
So this doesn't actually fix any of that stuff yet because the PPS code
doesn't handle two PPS for ADL yet.
Other than that,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 2 +-
> drivers/gpu/drm/i915/display/intel_bios.c | 56 ++++++++++++++-----
> drivers/gpu/drm/i915/display/intel_bios.h | 11 ++--
> .../drm/i915/display/intel_display_types.h | 2 +-
> drivers/gpu/drm/i915/display/intel_dp.c | 7 ++-
> drivers/gpu/drm/i915/display/intel_lvds.c | 4 +-
> drivers/gpu/drm/i915/display/intel_panel.c | 1 +
> drivers/gpu/drm/i915/display/intel_sdvo.c | 2 +-
> drivers/gpu/drm/i915/display/vlv_dsi.c | 2 +-
> 9 files changed, 61 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index e05e7cd6c412..b6b48d25f31e 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -2042,7 +2042,7 @@ void icl_dsi_init(struct drm_i915_private *dev_priv)
> /* attach connector to encoder */
> intel_connector_attach_encoder(intel_connector, encoder);
>
> - intel_bios_init_panel(dev_priv, &intel_connector->panel, NULL, NULL);
> + intel_bios_init_panel_late(dev_priv, &intel_connector->panel, NULL, NULL);
>
> mutex_lock(&dev_priv->drm.mode_config.mutex);
> intel_panel_add_vbt_lfp_fixed_mode(intel_connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index c2987f2c2b2e..64f927f6479d 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -620,14 +620,14 @@ static void dump_pnp_id(struct drm_i915_private *i915,
>
> static int opregion_get_panel_type(struct drm_i915_private *i915,
> const struct intel_bios_encoder_data *devdata,
> - const struct edid *edid)
> + const struct edid *edid, bool use_fallback)
> {
> return intel_opregion_get_panel_type(i915);
> }
>
> static int vbt_get_panel_type(struct drm_i915_private *i915,
> const struct intel_bios_encoder_data *devdata,
> - const struct edid *edid)
> + const struct edid *edid, bool use_fallback)
> {
> const struct bdb_lvds_options *lvds_options;
>
> @@ -652,7 +652,7 @@ static int vbt_get_panel_type(struct drm_i915_private *i915,
>
> static int pnpid_get_panel_type(struct drm_i915_private *i915,
> const struct intel_bios_encoder_data *devdata,
> - const struct edid *edid)
> + const struct edid *edid, bool use_fallback)
> {
> const struct bdb_lvds_lfp_data *data;
> const struct bdb_lvds_lfp_data_ptrs *ptrs;
> @@ -701,9 +701,9 @@ static int pnpid_get_panel_type(struct drm_i915_private *i915,
>
> static int fallback_get_panel_type(struct drm_i915_private *i915,
> const struct intel_bios_encoder_data *devdata,
> - const struct edid *edid)
> + const struct edid *edid, bool use_fallback)
> {
> - return 0;
> + return use_fallback ? 0 : -1;
> }
>
> enum panel_type {
> @@ -715,13 +715,13 @@ enum panel_type {
>
> static int get_panel_type(struct drm_i915_private *i915,
> const struct intel_bios_encoder_data *devdata,
> - const struct edid *edid)
> + const struct edid *edid, bool use_fallback)
> {
> struct {
> const char *name;
> int (*get_panel_type)(struct drm_i915_private *i915,
> const struct intel_bios_encoder_data *devdata,
> - const struct edid *edid);
> + const struct edid *edid, bool use_fallback);
> int panel_type;
> } panel_types[] = {
> [PANEL_TYPE_OPREGION] = {
> @@ -744,7 +744,8 @@ static int get_panel_type(struct drm_i915_private *i915,
> int i;
>
> for (i = 0; i < ARRAY_SIZE(panel_types); i++) {
> - panel_types[i].panel_type = panel_types[i].get_panel_type(i915, devdata, edid);
> + panel_types[i].panel_type = panel_types[i].get_panel_type(i915, devdata,
> + edid, use_fallback);
>
> drm_WARN_ON(&i915->drm, panel_types[i].panel_type > 0xf &&
> panel_types[i].panel_type != 0xff);
> @@ -3183,15 +3184,27 @@ void intel_bios_init(struct drm_i915_private *i915)
> kfree(oprom_vbt);
> }
>
> -void intel_bios_init_panel(struct drm_i915_private *i915,
> - struct intel_panel *panel,
> - const struct intel_bios_encoder_data *devdata,
> - const struct edid *edid)
> +static void intel_bios_init_panel(struct drm_i915_private *i915,
> + struct intel_panel *panel,
> + const struct intel_bios_encoder_data *devdata,
> + const struct edid *edid,
> + bool use_fallback)
> {
> + /* already have it? */
> + if (panel->vbt.panel_type >= 0) {
> + drm_WARN_ON(&i915->drm, !use_fallback);
> + return;
> + }
> +
> + panel->vbt.panel_type = get_panel_type(i915, devdata,
> + edid, use_fallback);
> + if (panel->vbt.panel_type < 0) {
> + drm_WARN_ON(&i915->drm, use_fallback);
> + return;
> + }
> +
> init_vbt_panel_defaults(panel);
>
> - panel->vbt.panel_type = get_panel_type(i915, devdata, edid);
> -
> parse_panel_options(i915, panel);
> parse_generic_dtd(i915, panel);
> parse_lfp_data(i915, panel);
> @@ -3205,6 +3218,21 @@ void intel_bios_init_panel(struct drm_i915_private *i915,
> parse_mipi_sequence(i915, panel);
> }
>
> +void intel_bios_init_panel_early(struct drm_i915_private *i915,
> + struct intel_panel *panel,
> + const struct intel_bios_encoder_data *devdata)
> +{
> + intel_bios_init_panel(i915, panel, devdata, NULL, false);
> +}
> +
> +void intel_bios_init_panel_late(struct drm_i915_private *i915,
> + struct intel_panel *panel,
> + const struct intel_bios_encoder_data *devdata,
> + const struct edid *edid)
> +{
> + intel_bios_init_panel(i915, panel, devdata, edid, true);
> +}
> +
> /**
> * intel_bios_driver_remove - Free any resources allocated by intel_bios_init()
> * @i915: i915 device instance
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
> index e375405a7828..ff1fdd2e0c1c 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> @@ -232,10 +232,13 @@ 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,
> - struct intel_panel *panel,
> - const struct intel_bios_encoder_data *devdata,
> - const struct edid *edid);
> +void intel_bios_init_panel_early(struct drm_i915_private *dev_priv,
> + struct intel_panel *panel,
> + const struct intel_bios_encoder_data *devdata);
> +void intel_bios_init_panel_late(struct drm_i915_private *dev_priv,
> + struct intel_panel *panel,
> + const struct intel_bios_encoder_data *devdata,
> + const struct edid *edid);
> void intel_bios_fini_panel(struct intel_panel *panel);
> void intel_bios_driver_remove(struct drm_i915_private *dev_priv);
> bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index c6abaaa46e17..aec06cb24e23 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -291,7 +291,7 @@ struct intel_vbt_panel_data {
> struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
>
> /* Feature bits */
> - unsigned int panel_type:4;
> + int panel_type;
> unsigned int lvds_dither:1;
> unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7400d6b4c587..509ce003312b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> return false;
> }
>
> + intel_bios_init_panel_early(dev_priv, &intel_connector->panel,
> + encoder->devdata);
> +
> intel_pps_init(intel_dp);
>
> /* Cache DPCD and EDID for edp. */
> @@ -5289,8 +5292,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> }
> intel_connector->edid = edid;
>
> - intel_bios_init_panel(dev_priv, &intel_connector->panel,
> - encoder->devdata, IS_ERR(edid) ? NULL : edid);
> + intel_bios_init_panel_late(dev_priv, &intel_connector->panel,
> + encoder->devdata, IS_ERR(edid) ? NULL : edid);
>
> intel_panel_add_edid_fixed_modes(intel_connector, true);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> index 246787bbf5ef..e19ae171af33 100644
> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> @@ -963,8 +963,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> }
> intel_connector->edid = edid;
>
> - intel_bios_init_panel(dev_priv, &intel_connector->panel, NULL,
> - IS_ERR(edid) ? NULL : edid);
> + intel_bios_init_panel_late(dev_priv, &intel_connector->panel, NULL,
> + IS_ERR(edid) ? NULL : edid);
>
> /* Try EDID first */
> intel_panel_add_edid_fixed_modes(intel_connector, true);
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index 8e683ab75594..918b3b9d9ebe 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -664,6 +664,7 @@ void intel_panel_init_alloc(struct intel_connector *connector)
> {
> struct intel_panel *panel = &connector->panel;
>
> + connector->panel.vbt.panel_type = -1;
> INIT_LIST_HEAD(&panel->fixed_modes);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 48b7b1aa37b2..b2122aff3f63 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2885,7 +2885,7 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, u16 type)
> if (!intel_sdvo_create_enhance_property(intel_sdvo, intel_sdvo_connector))
> goto err;
>
> - intel_bios_init_panel(i915, &intel_connector->panel, NULL, NULL);
> + intel_bios_init_panel_late(i915, &intel_connector->panel, NULL, NULL);
>
> /*
> * Fetch modes from VBT. For SDVO prefer the VBT mode since some
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 5a741ea4505f..d84352bc1376 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1915,7 +1915,7 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv)
>
> intel_dsi->panel_power_off_time = ktime_get_boottime();
>
> - intel_bios_init_panel(dev_priv, &intel_connector->panel, NULL, NULL);
> + intel_bios_init_panel_late(dev_priv, &intel_connector->panel, NULL, NULL);
>
> if (intel_connector->panel.vbt.dsi.config->dual_link)
> intel_dsi->ports = BIT(PORT_A) | BIT(PORT_C);
--
Jani Nikula, Intel Open Source Graphics Center
next prev parent reply other threads:[~2022-11-09 14:59 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-09 11:16 [Intel-gfx] [PATCH 0/6] drm/i915: Fake dual eDP VBT fixes Ville Syrjala
2022-11-09 11:16 ` [Intel-gfx] [PATCH 1/6] drm/i915: Introduce intel_panel_init_alloc() Ville Syrjala
2022-11-09 14:49 ` Jani Nikula
2022-11-09 11:16 ` [Intel-gfx] [PATCH 2/6] drm/i915: Do panel VBT init early if the VBT declares an explicit panel type Ville Syrjala
2022-11-09 14:59 ` Jani Nikula [this message]
2022-11-09 11:16 ` [Intel-gfx] [PATCH 3/6] drm/i915: Generalize the PPS vlv_pipe_check() stuff Ville Syrjala
2022-11-09 15:24 ` Jani Nikula
2022-11-09 11:16 ` [Intel-gfx] [PATCH 4/6] drm/i915: Try to use the correct power sequencer intiially on bxt/glk Ville Syrjala
2022-11-10 13:56 ` Manna, Animesh
2022-11-10 14:41 ` Ville Syrjälä
2022-11-10 19:30 ` Ville Syrjälä
2022-11-09 11:16 ` [Intel-gfx] [PATCH 5/6] drm/915: Extend dual PPS handlind for ICP+ Ville Syrjala
2022-11-10 14:09 ` Manna, Animesh
2022-11-10 14:45 ` Ville Syrjälä
2022-11-09 11:16 ` [Intel-gfx] [PATCH 6/6] drm/i915: Ignore LFP2 for now Ville Syrjala
2022-11-09 11:31 ` Ville Syrjälä
2022-11-09 14:03 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Fake dual eDP VBT fixes Patchwork
2022-11-09 14:25 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-11-09 19:22 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=875yfo6uo1.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox